generated from tc39/template-for-proposals
-
Notifications
You must be signed in to change notification settings - Fork 11
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove importMetaHook #54
Comments
If we remove this, we'll lost the ability of doing #13 |
kriskowal
added a commit
that referenced
this issue
Jun 14, 2022
Fixes #54 The importMetaHook is superfluous if static module records reveal needsImportMetaHook. The decision of whether to build out importMeta can be made in loadHook.
Merged
@Jack-Works Per #13 (comment), I would like to for now ignore the motivating use-case for revealing the module environment record or module exports namespace on |
I think it's OK for now. |
kriskowal
added a commit
that referenced
this issue
Jul 12, 2022
Fixes #54 The importMetaHook is superfluous if static module records reveal needsImportMetaHook. The decision of whether to build out importMeta can be made in loadHook.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
@patrick-soquet notes that we can remove the complication of an
importMetaHook
by instead reflectingneedsImportMeta
onStaticModuleRecord
instances. This is consistent with the property we already expect in theSyntheticStaticModuleRecord
protocol. By reflectingneedsImportMeta
, we can move the concern toloadHook
. The implementer of aloadHook
is in a position to decide whether or not to elaborate theimportMeta
they attach to a module descriptor, based on the presence ofneedsImportMeta
on the underlyingStaticModuleRecord
.The text was updated successfully, but these errors were encountered: