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

process: clarify the pre- and post-condition of esm setup #25530

Closed
wants to merge 1 commit into from

Conversation

joyeecheung
Copy link
Member

This patch:

  • Clarifies the dependency of the ESM loader initialization
    (process.cwd() and the value of --loader) in node.js.
  • Moves the initialization of the per-isolate importModuleDynamically
    and initializeImportMetaObject callbacks into node.js
  • Moves the initialization of the ESM loader into
    prepareUserCodeExecution() since it potentially involves
    execution of user code (similar to --require for CJS modules).
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

This patch:

- Clarifies the dependency of the ESM loader initialization
  (`process.cwd()` and the value of `--loader`) in `node.js`.
- Moves the initialization of the per-isolate `importModuleDynamically`
  and `initializeImportMetaObject` callbacks into `node.js`
- Moves the initialization of the ESM loader into
  `prepareUserCodeExecution()` since it potentially involves
  execution of user code (similar to `--require` for CJS modules).
@nodejs-github-bot nodejs-github-bot added the process Issues and PRs related to the process subsystem. label Jan 16, 2019
@joyeecheung
Copy link
Member Author

@joyeecheung
Copy link
Member Author

According to the GitHub reviewer recommendation, @devsnek @guybedford @addaleax PTAL. This helps moving the first loading of 'internal/modules/cjs/loader' into prepareUserCodeExecution so that we can split the execution mode selection from bootstrapping (getting the environment ready).

@joyeecheung
Copy link
Member Author

joyeecheung added a commit that referenced this pull request Jan 23, 2019
This patch:

- Clarifies the dependency of the ESM loader initialization
  (`process.cwd()` and the value of `--loader`) in `node.js`.
- Moves the initialization of the per-isolate `importModuleDynamically`
  and `initializeImportMetaObject` callbacks into `node.js`
- Moves the initialization of the ESM loader into
  `prepareUserCodeExecution()` since it potentially involves
  execution of user code (similar to `--require` for CJS modules).

PR-URL: #25530
Reviewed-By: Gus Caplan <[email protected]>
@joyeecheung
Copy link
Member Author

Landed in a93c825

addaleax pushed a commit that referenced this pull request Jan 23, 2019
This patch:

- Clarifies the dependency of the ESM loader initialization
  (`process.cwd()` and the value of `--loader`) in `node.js`.
- Moves the initialization of the per-isolate `importModuleDynamically`
  and `initializeImportMetaObject` callbacks into `node.js`
- Moves the initialization of the ESM loader into
  `prepareUserCodeExecution()` since it potentially involves
  execution of user code (similar to `--require` for CJS modules).

PR-URL: #25530
Reviewed-By: Gus Caplan <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Jan 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
process Issues and PRs related to the process subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants