proposal-defer-import-eval icon indicating copy to clipboard operation
proposal-defer-import-eval copied to clipboard

Stage 2.7 roadmap

Open nicolo-ribaudo opened this issue 1 year ago • 9 comments

Before Stage 3, we still need to investigate multiple aspects of the proposal. Each topic should be discussed in its own issue, but I'm writing all of them here to keep track of the progress.


TODOs

  • [x] Interop with WASM modules (https://github.com/tc39/proposal-defer-import-eval/issues/22) The current WASM-ESM integration proposal makes WASM modules async from JS's perspective. This meant that their execution would never be deferred. Can this be changed?
  • [x] More performance analysis We need to investigate more the performance benefits of this proposal, and collect more data. Additionally, we need to explain why top-level await suddenly causing a module to not be deferred anymore is not a performance footgun.
  • [ ] ~~Early errors (related to https://github.com/tc39/proposal-defer-import-eval/issues/9)~~ How critical are early errors? Is it ok to defer them until execution, similarly to what happens with dynamic import? This would allow some systems to entirely defer loading modules. EDIT: After talking with people working on various runtime implementations, it's clear that those implementations would still be able to avoid loading by pre-computing metadata about files.
  • [ ] ~~Deferred re-exports (related to https://github.com/tc39/proposal-defer-import-eval/issues/18)~~ (#30, #31) It would be useful to support something like export defer { f } from "./foo", that causes the execution of ./foo only if f is actually imported form this module. EDIT: The surface area for re-exports is much larger due to the tree-shaking semantics, I plan to ask the committee to leave it at stage 2 for longer.
  • [x] Dynamic import syntax (PR: https://github.com/tc39/proposal-defer-import-eval/pull/28) Given that import source is now stage 3, we should probably add dynamic import.defer() to this proposal similar to import.source().

Reviews

  • [x] @guybedford (note: Guy was initially a co-author of the proposal with Yulia, but both of them stepped down from active development long ago during Stage 1)
  • [x] @dminor
  • Editors
    • [ ] @bakkot
    • [x] @michaelficarra
    • [ ] @syg

nicolo-ribaudo avatar Sep 11 '23 12:09 nicolo-ribaudo

  • [ ] @bakkot
  • [ ] @michaelficarra
  • [ ] @syg

I was planning on proposing this for Stage 2.7 at the April meeting, and I remembered just a few days ago (because I was pinged for a review) that editor reviews are needed for Stage 2.7 and not for Stage 3.

The spec text at https://tc39.es/proposal-defer-import-eval/ is complete and is what I plan to propose. Regarding the open PRs in this repo:

  • #34 is for a possible minor change to how evaluation happens that I need to run through the committee, mostly because I don't have a preference on whether we should do it or not.
  • #30&#31 are something that I will ask to the committee to "leave behind" as a separate stage 2 proposal, because while working on them I realized that they are very different from import defer. They can be thus ignored.

I know that this ping is coming way too late, so I understand if it's impossible for you to take a look before plenary :)

nicolo-ribaudo avatar Apr 01 '24 17:04 nicolo-ribaudo

My review can be considered complete after https://github.com/tc39/proposal-defer-import-eval/issues/38 is resolved, with the PR in https://github.com/tc39/proposal-defer-import-eval/pull/39 and https://github.com/tc39/proposal-defer-import-eval/pull/35 already landed. The implementation approach is really neat, nice work.

guybedford avatar Apr 02 '24 20:04 guybedford

This looks good to me, thank you for the good work here.

dminor avatar Apr 04 '24 19:04 dminor

@bakkot @michaelficarra @syg I'm planning to re-propose this for 2.7 at the next meeting, I would appreciate if you could find some time to review the spec text.

If you already took a look at this last time, there have been two PRs to handle a bug that @guybedford found with re-entrancy and top-level errors: https://github.com/tc39/proposal-defer-import-eval/pull/39 and https://github.com/tc39/proposal-defer-import-eval/pull/43

nicolo-ribaudo avatar May 21 '24 20:05 nicolo-ribaudo

I've had another look, I think this is good for 2.7.

dminor avatar May 30 '24 13:05 dminor

@nicolo-ribaudo Why do you use an early error to restrict ImportClause in ImportDeclaration to NameSpaceImport when defer is present? Why not just add an alternative to ImportDeclaration?

michaelficarra avatar Jun 06 '24 20:06 michaelficarra

@nicolo-ribaudo There's also a misspelling of [[Specifer]] that occurs multiple times.

edit: I'll just turn this into a review feedback comment.

  • evaluatePromise doesn't need to be declared in step 8.c. You can have more than one declaration (at the points where you currently assign it) as long as all paths to the first use pass through exactly one declaration.
  • I don't like that we're using an out parameter in GatherAsynchronousTransitiveDependencies. It looks like we can easily avoid it with some list-concatenations, right?

michaelficarra avatar Jun 06 '24 20:06 michaelficarra

@nicolo-ribaudo If you don't mind me asking, is 2.7 an arbitrary number between 2 and 3? Why "Stage 2.7" and not, for example, "Stage 2.5", or "Stage 2.1", or "Stage 2.9"?

parzhitsky avatar Sep 02 '24 10:09 parzhitsky

@parzhitsky You can read the discussion at https://github.com/tc39/notes/blob/main/meetings/2023-11/november-30.md#continuation-of-the-new-stage-discussion

nicolo-ribaudo avatar Sep 02 '24 12:09 nicolo-ribaudo