meteor / reify

Enable ECMAScript 2015 modules in Node today. No caveats. Full stop.
MIT License
6 stars 3 forks source link

TLA detection trips on reify’s own injected code #11

Open perbergland opened 1 week ago

perbergland commented 1 week ago

While investigating https://github.com/meteor/meteor/issues/13424, I updated https://github.com/Meteor-Community-Packages/meteor-typescript to publish against 3.0.4 so that reify 0.25.3 was brought in.

Using that version now correctly flags code like top level const x = await methodCall() as TLA.

Unfortunately, it also flags a lot of modules that DON’T have any TLA code as TLA and this is problematic since it forces adding "await" to internal require calls.

This problem was made visible in the https://github.com/meteor/meteor/pull/13298/files PR where the author had to add "await" to some require statements that shouldn’t have needed this.

perbergland commented 1 week ago

It’s probably something else amiss.

I can’t get reify to mark a single module as async: false

Testing with this sample project: https://github.com/Meteor-Community-Packages/meteor-typescript/tree/master/tests/small-typescript-app-meteor3

There is no occurrence of async: false in tests/small-typescript-app-meteor3/.meteor/local/build/programs/server/app/app.js

perbergland commented 1 week ago

I found the specific problem when reify was invoked via the refapp:meteor-typescript package which subclasses BabelCompiler in the same way that the meteor core Typescript does:

reify triggers TLA detection for the AST that it is adding itself

The code that triggers the TLA detection turned out the be:

if (__reifyWaitForDeps__()) (await __reifyWaitForDeps__())();

This code is added BEFORE the TLA checking whereas in the unit tests in this repo, that doesn’t seem to be the case.