[TS/JS] New gen TS code gen #6302

bjornharrtell · 2020-12-01T21:24:59Z

Modernize TS/JS code gen by doing a reworked TS only code gen.

TODO

Documentation

Decide what to do about Compiler.md, JavaScriptUsage.md, Tutorial.md and javascript_sample.sh

TODO (undecided)

aardappel · 2020-12-03T20:28:51Z

@krojew

krojew · 2020-12-03T20:30:48Z

This is a good first step. Next would be splitting per table as java does?

bjornharrtell · 2020-12-03T20:33:28Z

@krojew yep might be, I've added a TODO list in the issue description.

krojew · 2020-12-14T16:36:17Z

How's it going? Do you need any help?

bjornharrtell · 2020-12-14T20:31:02Z

@krojew afraid I still haven't gotten free time for it, if you want to get started now please do! I aim to get some alone and free time in xmax holidays to possibly pick this up so somewhere 25-30 december. Keep me in the loop. :)

bjornharrtell · 2020-12-26T16:06:48Z

@krojew will try to get somewhere with this today. What's your status?

krojew · 2020-12-26T16:31:36Z

I haven't managed to get any free time to work on this yet. But at least, I think we know how to proceed - remove JS support and output a file per table, as Java does. This implies no more --get-all support, but I don't think it's a problem.

bjornharrtell · 2020-12-27T16:38:18Z

I'm slowly realizing this will require more than just simple tweaks to existing code and I'm unfortunately quite far from being able to get somewhere. Ideally I would like to have a better setup/testing story than to compile, run the flatc binary and inspect output.

bjornharrtell · 2020-12-27T16:49:28Z

@krojew we opted to use kebab-case in #6095. When generating code all other languages seem to use PascalCase as folder and file names. I wonder if it would not be better to use PascalCase for TS too, for consistency and also to some extent simplicity?

krojew · 2020-12-27T16:55:04Z

I think we should keep kebab case, since we would cause problems for Angular users otherwise.

bjornharrtell · 2020-12-27T18:17:28Z

@krojew ok I have no strong opinion so agreed.

bjornharrtell · 2020-12-27T19:41:29Z

@krojew getting somewhere, now generating separate files in kebab case. (still large amount of work remaining on actual code output though)

bjornharrtell · 2020-12-27T19:45:27Z

For future reference this is what imports could look like:

import { SIZE_PREFIX_LENGTH } from '../../js/constants'
import { Long, createLong } from '../../js/long'
import { Offset, Table } from '../../js/types'
import { Builder } from '../../js/builder'
import { ByteBuffer } from '../../js/byte-buffer'
import { Encoding } from '../../js/encoding'

bjornharrtell · 2020-12-27T19:48:04Z

@krojew any ideas on how to alias flatbuffers imports? A flatbuffers_ prefix or simply _ perhaps?

krojew · 2020-12-27T20:12:02Z

I suggest _flatbuffers_, since it's both unique and obvious.

bjornharrtell · 2020-12-28T12:15:05Z

@krojew output looks promising now however not really tested at this point. Feel free to do an early review.

bjornharrtell · 2020-12-28T12:49:44Z

@krojew instead of _flatbuffers_ prefix we could reexport all of flatbuffers public API in a single entry point flatbuffers module and import with * as flatbuffers from it.

krojew

We should also remove JS-specific flags.

krojew · 2020-12-28T13:28:26Z

tests/tsconfig.json

+    "target": "ES6",
+    "lib": ["ES2015", "ES2020.BigInt", "DOM"],
+    "noImplicitAny": false,
+    "strict": false,


We should test for maxium strict support.

src/idl_gen_js_ts.cpp

bjornharrtell · 2020-12-28T14:22:06Z

@krojew I'm not 100% confident about my approach to import tracking and namespace aliasing when needed which replaces the need for prefix/namespace wrappings. But I've just now verified it seems to be working in my external project where the end result bundle ends up almost 40% smaller so that's a nice side-effect of this work.

krojew · 2020-12-28T14:23:31Z

I would need to see regenerated test files to have an opinion. Right now, it's hard to see what the actual output is.

This reverts commit 6f4eb01.

This reverts commit 676b213.

This reverts commit 2d08648.

bjornharrtell · 2021-01-14T19:54:19Z

Rebased with more success (I sure hope so).

krojew · 2021-01-15T14:31:17Z

@aardappel can you merge?

krojew · 2021-01-19T07:00:37Z

@aardappel any news on this?

aardappel · 2021-01-19T20:51:09Z

Thanks for all the hard work :)

dbaileychess · 2022-04-13T02:47:41Z

src/flatc.cpp

        opts.natural_utf8 = true;
-      } else if (arg == "--no-js-exports") {
-        opts.skip_js_exports = true;
-      } else if (arg == "--goog-js-export") {


How does this work now? I have a bunch of users internally who are setting this flag and it no longer is there.

@bjornharrtell it is hard for me to see what changed here since it such a big merge. how does --goog-js-export work now that the flag is removed?

@aardappel

Yes, it seems unwise to have removed that flag or any code related to it, since we couldn't test that externally. Apologies if I didn't spot that during this huge PR and huge review..

But this has been merged for a while now, so all we can do is try and fix this forward.

Memory is vague at this point but I think the general idea was that TS is the only target and exports using es modules. If someone needs another module format they'd have to transpile it themselves.

bjornharrtell mentioned this pull request Dec 1, 2020

[TS] Codegen should import from individual modules #6145

Closed

bjornharrtell force-pushed the ts-spike branch from de8b0bd to 707d2c1 Compare December 26, 2020 19:35

github-actions bot added c++ codegen Involving generating code from schema javascript json typescript labels Dec 26, 2020

bjornharrtell changed the title ~~[TS/JS] TS/ES6 modules spike~~ [TS/JS] New gen TS code gen Dec 27, 2020

bjornharrtell mentioned this pull request Dec 28, 2020

[CPP] Formats cpp files with clang-format #6349

Merged

krojew reviewed Dec 28, 2020

View reviewed changes

This comment has been minimized.

Sign in to view

bjornharrtell added 19 commits January 14, 2021 20:52

Use CharToLower

774ea51

Use normal for loop

511d710

Rework namespacedir

ef8919d

Revert "Rework namespacedir"

bf6b48e

This reverts commit 6f4eb01.

Revert "Use normal for loop"

c7cf83c

This reverts commit 676b213.

Revert "Use CharToLower"

06ebebe

This reverts commit 2d08648.

Again do rework but correct

88d5e53

Avoid runtime cast

0b3b019

Fix test runs

4a32259

Also add npm install to get tsc

c5d1df1

Bump node test versions

9a9412b

for range to std for loop

392f82c

Clang format

f1f7089

Missed one clang format

5735dba

Move accessor to later

49dc234

Attempt to make windows version of TypeScriptTest

7f5e859

Want to see the output

b99ea49

Try to get newer node at appveyor

ad01a68

Style changes

638b5e4

bjornharrtell force-pushed the ts-spike branch from 82cee47 to 638b5e4 Compare January 14, 2021 19:53

aardappel merged commit 760c657 into google:master Jan 19, 2021

LucasSwitz mentioned this pull request Jan 19, 2021

[AssemblyScript] Added basic AssemblyScript support #6408

Closed

bjornharrtell deleted the ts-spike branch January 19, 2021 22:06

mustiikhalil mentioned this pull request Feb 4, 2021

[TS] Fixes GRPC code #6441

Merged

dbaileychess reviewed Apr 13, 2022

View reviewed changes

fawdlstty mentioned this pull request Dec 23, 2025

Why not support --js complier option in the newest version ? #8518

Closed

[TS/JS] New gen TS code gen #6302

[TS/JS] New gen TS code gen #6302

Uh oh!

Conversation

bjornharrtell commented Dec 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

TODO

Documentation

TODO (undecided)

Uh oh!

aardappel commented Dec 3, 2020

Uh oh!

krojew commented Dec 3, 2020

Uh oh!

bjornharrtell commented Dec 3, 2020

Uh oh!

krojew commented Dec 14, 2020

Uh oh!

bjornharrtell commented Dec 14, 2020

Uh oh!

bjornharrtell commented Dec 26, 2020

Uh oh!

krojew commented Dec 26, 2020

Uh oh!

bjornharrtell commented Dec 27, 2020

Uh oh!

bjornharrtell commented Dec 27, 2020

Uh oh!

krojew commented Dec 27, 2020

Uh oh!

bjornharrtell commented Dec 27, 2020

Uh oh!

bjornharrtell commented Dec 27, 2020

Uh oh!

bjornharrtell commented Dec 27, 2020

Uh oh!

bjornharrtell commented Dec 27, 2020

Uh oh!

krojew commented Dec 27, 2020

Uh oh!

bjornharrtell commented Dec 28, 2020

Uh oh!

bjornharrtell commented Dec 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

krojew left a comment

Choose a reason for hiding this comment

Uh oh!

krojew Dec 28, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bjornharrtell commented Dec 28, 2020

Uh oh!

krojew commented Dec 28, 2020

Uh oh!

This comment has been minimized.

bjornharrtell commented Jan 14, 2021

Uh oh!

krojew commented Jan 15, 2021

Uh oh!

krojew commented Jan 19, 2021

Uh oh!

aardappel commented Jan 19, 2021

Uh oh!

dbaileychess Apr 13, 2022

Choose a reason for hiding this comment

Uh oh!

dbaileychess Apr 13, 2022

Choose a reason for hiding this comment

Uh oh!

aardappel Apr 13, 2022

Choose a reason for hiding this comment

Uh oh!

bjornharrtell Apr 13, 2022

Choose a reason for hiding this comment

Uh oh!

bjornharrtell commented Dec 1, 2020 •

edited

Loading

bjornharrtell commented Dec 28, 2020 •

edited

Loading