Conversation
|
The rendered spec for this PR is available at https://endojs.github.io/proposal-import-hook/pr/1. |
That gives me a 404 |
| # template-for-proposals | ||
|
|
||
| A repository template for ECMAScript proposals. |
There was a problem hiding this comment.
Is this guide to contributing a proposal mistakenly copied from the template?
There was a problem hiding this comment.
Rather than delete it, I moved it out of the way.
There was a problem hiding this comment.
totally fine to delete it too, the content's visible on the template and in the history :-p
| > The handler is necessary for capturing a base specifier for resolving a | ||
| > relative import specifier, and allows 262 to avoid unnecssary specificity | ||
| > about resolution algorithms. |
There was a problem hiding this comment.
Why quoted? What is it quoting? (Likewise for other uses of > )
There was a problem hiding this comment.
It’s parenthetical commentary.
| importHook?: ImportHook, | ||
| importMetaHook?: ImportMetaHook, | ||
| // ... | ||
| [name: string | symbol | number]: unknown, |
There was a problem hiding this comment.
Would not actually be part of a TypeScript type for typing what this proposal proposes, yes?
There was a problem hiding this comment.
This is intended to communicate that the handler may have other, unspecified properties that are communicated as normal through to the receivers of the hooks.
| > For consideration: for a dynamic import, we may or may not pass the given | ||
| > import attributes object through to import hooks, and we may or may not | ||
| > assert or otherwise adjust the key order to be canonicalized. | ||
| > The canonical order is important for the implementation to form a | ||
| > deterministic cache key from arbitrary attributes but an import hook should | ||
| > not be sensitive to that order. |
There was a problem hiding this comment.
This sounds like a crazy degree of non-determinism to allow an implementation. Can we pin this down in the obvious way?
There was a problem hiding this comment.
We have that option, yes. I wrote it both ways, so it’s a good point to raise in an issue for discussion.
|
|
||
| > The Module Harmony group elected to avoid deferring the import meta hook | ||
| > to the first evaluation of an `import.meta` expression, but leaves the option | ||
| > open for a host hook, just not a user code hook. |
There was a problem hiding this comment.
Breaking host virtualizability? This seems bad.
There was a problem hiding this comment.
Hosts do not currently do anything with the privilege of deferring creation of import.meta that is observably different than populating import.meta before execution, except that they are able to avoid the allocation and population of the import.meta in cases where import.meta is uttered in the source but never executed.
| for the corresponding module namespace. | ||
| In that process, the implementation will call `importHook` for each module | ||
| imported statically and dynamically in that module, giving it the module | ||
| specifier string an canonicalized import attributes. |
README.md
Outdated
| ## New Global | ||
|
|
||
| This import hooks proposal creates a foundation for a [`new Global` | ||
| proposal](proposal-new-global), which in turn provides a sufficient foundation |
There was a problem hiding this comment.
The rendered link is broken
| proposal](proposal-new-global), which in turn provides a sufficient foundation | |
| proposal](https://github.com/endojs/proposal-new-global/), which in turn provides a sufficient foundation |
until it moves to a better place?
There was a problem hiding this comment.
The errors is using parens rather than brackets.
README.md
Outdated
|
|
||
| This import hooks proposal creates a foundation for a [`new Global` | ||
| proposal](proposal-new-global), which in turn provides a sufficient foundation | ||
| to implement [`Compartment` proposal](proposal-compartments) and |
There was a problem hiding this comment.
| to implement [`Compartment` proposal](proposal-compartments) and | |
| to implement [`Compartment` proposal](https://github.com/tc39/proposal-compartments) and |
| We expect a future proposal to introduce `import.now` or `import.sync` as a | ||
| mechanism for synchronously growing the module graph and executing a module. | ||
| If so, `ModuleSource` handlers would need a corresponding synchronous import | ||
| hook that may not return a promise. |
There was a problem hiding this comment.
| hook that may not return a promise. | |
| hook that must not return a promise. |
?
| going async there has no implications whatsoever. | ||
| In prior iterations of this, the user was responsible for loop thru the | ||
| dependencies, and prepare the instance before kicking the next phase, that's | ||
| not longer the case here, where the level of control on the different phases is |
There was a problem hiding this comment.
| not longer the case here, where the level of control on the different phases is | |
| no longer the case here, where the level of control on the different phases is |
|
|
||
| Since the `importHook` is only triggered via the kicker (`import(instance)`), | ||
| going async there has no implications whatsoever. | ||
| In prior iterations of this, the user was responsible for loop thru the |
There was a problem hiding this comment.
| In prior iterations of this, the user was responsible for loop thru the | |
| In prior iterations of this, the user was responsible for looping thru the |
| Since the `importHook` is only triggered via the kicker (`import(instance)`), | ||
| going async there has no implications whatsoever. | ||
| In prior iterations of this, the user was responsible for loop thru the | ||
| dependencies, and prepare the instance before kicking the next phase, that's |
There was a problem hiding this comment.
| dependencies, and prepare the instance before kicking the next phase, that's | |
| dependencies, and preparing the instance before kicking the next phase. That's |
|
|
||
| ### Can cycles be represented? | ||
|
|
||
| Yes, `importHook` can return a `Module` that was either `import()` already or |
There was a problem hiding this comment.
| Yes, `importHook` can return a `Module` that was either `import()` already or | |
| Yes, `importHook` can return a `Module` that was either `import()`ed already or |
erights
left a comment
There was a problem hiding this comment.
Approving because, even without my suggestions, it is more than good enough to point to from the draft agenda before deadline. All suggestions can be addressed after.
This migrates and updates the explainer for what was Compartments Layer 0: Module and ModuleSource. The new proposal drops the parts of Source Phase Imports and ESM Phase Imports that have already progressed to Stage 2.7 and consolidates the
ModuleandModuleInstance, with an additional design rationale section explaining why I am personally favoring that direction.