Skip to content
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

lib: add assertion for user ESM execution #51748

Merged
merged 1 commit into from
Feb 21, 2024

Conversation

joyeecheung
Copy link
Member

Previously we only had an internal assertion to ensure certain code is executed before any user-provided CJS is run. This patch adds another assertion for ESM.

Note that this internal state is not updated during source text module execution via vm because to run any code via vm, some user JS code must have already been executed anyway.

In addition this patch moves the states into internal/modules/helpers to avoid circular dependencies. Also moves toggling the states to true right before user code execution instead of after in case we are half-way in the execution when internals try to check them.

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/loaders

@nodejs-github-bot nodejs-github-bot added esm Issues and PRs related to the ECMAScript Modules implementation. needs-ci PRs that need a full CI run. process Issues and PRs related to the process subsystem. labels Feb 13, 2024
Previously we only had an internal assertion to ensure certain
code is executed before any user-provided CJS is run. This patch
adds another assertion for ESM.

Note that this internal state is not updated during source text
module execution via vm because to run any code via vm, some
user JS code must have already been executed anyway.

In addition this patch moves the states into internal/modules/helpers
to avoid circular dependencies. Also moves toggling the states to
true *right before* user code execution instead of after in case
we are half-way in the execution when internals try to check them.
@joyeecheung joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 17, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 17, 2024
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@joyeecheung joyeecheung added the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 21, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 21, 2024
@nodejs-github-bot nodejs-github-bot merged commit 0951e7b into nodejs:main Feb 21, 2024
55 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 0951e7b

marco-ippolito pushed a commit that referenced this pull request Feb 26, 2024
Previously we only had an internal assertion to ensure certain
code is executed before any user-provided CJS is run. This patch
adds another assertion for ESM.

Note that this internal state is not updated during source text
module execution via vm because to run any code via vm, some
user JS code must have already been executed anyway.

In addition this patch moves the states into internal/modules/helpers
to avoid circular dependencies. Also moves toggling the states to
true *right before* user code execution instead of after in case
we are half-way in the execution when internals try to check them.

PR-URL: #51748
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
marco-ippolito pushed a commit that referenced this pull request Feb 27, 2024
Previously we only had an internal assertion to ensure certain
code is executed before any user-provided CJS is run. This patch
adds another assertion for ESM.

Note that this internal state is not updated during source text
module execution via vm because to run any code via vm, some
user JS code must have already been executed anyway.

In addition this patch moves the states into internal/modules/helpers
to avoid circular dependencies. Also moves toggling the states to
true *right before* user code execution instead of after in case
we are half-way in the execution when internals try to check them.

PR-URL: #51748
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
@marco-ippolito marco-ippolito mentioned this pull request Mar 1, 2024
rdw-msft pushed a commit to rdw-msft/node that referenced this pull request Mar 20, 2024
Previously we only had an internal assertion to ensure certain
code is executed before any user-provided CJS is run. This patch
adds another assertion for ESM.

Note that this internal state is not updated during source text
module execution via vm because to run any code via vm, some
user JS code must have already been executed anyway.

In addition this patch moves the states into internal/modules/helpers
to avoid circular dependencies. Also moves toggling the states to
true *right before* user code execution instead of after in case
we are half-way in the execution when internals try to check them.

PR-URL: nodejs#51748
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
richardlau pushed a commit that referenced this pull request Mar 25, 2024
Previously we only had an internal assertion to ensure certain
code is executed before any user-provided CJS is run. This patch
adds another assertion for ESM.

Note that this internal state is not updated during source text
module execution via vm because to run any code via vm, some
user JS code must have already been executed anyway.

In addition this patch moves the states into internal/modules/helpers
to avoid circular dependencies. Also moves toggling the states to
true *right before* user code execution instead of after in case
we are half-way in the execution when internals try to check them.

PR-URL: #51748
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
richardlau pushed a commit that referenced this pull request Mar 25, 2024
Previously we only had an internal assertion to ensure certain
code is executed before any user-provided CJS is run. This patch
adds another assertion for ESM.

Note that this internal state is not updated during source text
module execution via vm because to run any code via vm, some
user JS code must have already been executed anyway.

In addition this patch moves the states into internal/modules/helpers
to avoid circular dependencies. Also moves toggling the states to
true *right before* user code execution instead of after in case
we are half-way in the execution when internals try to check them.

PR-URL: #51748
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
@richardlau richardlau mentioned this pull request Mar 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
esm Issues and PRs related to the ECMAScript Modules implementation. needs-ci PRs that need a full CI run. process Issues and PRs related to the process subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants