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.

Sign up for GitHub

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

Jump to bottom

[9.x] backport 19112, 19177 (bootstrapper & loaders reafactor) #19374

Closed
wants to merge 5 commits into from
Closed

[9.x] backport 19112, 19177 (bootstrapper & loaders reafactor) #19374

wants to merge 5 commits into from

Commits on Mar 30, 2018

  1. src: move internal loaders out of bootstrap_node.js 

    - Moves the creation of `process.binding()`, `process._linkedBinding()`
  `internalBinding()` and `NativeModule` into a separate file
  `lib/internal/bootstrap_loaders.js`, and documents them there.
  This file will be compiled and run before `bootstrap_node.js`, which
  means we now bootstrap the internal module & binding system before
  actually bootstrapping Node.js.
- Rename the special ID that can be used to require `NativeModule`
  as `internal/bootstrap_loaders` since it is setup there. Also put
  `internalBinding` in the object exported by `NativeModule.require`
  instead of putting it inside the `NativeModule.wrapper`
- Use the original `getBinding()` to get the source code of native
  modules instead of getting it from `process.binding('native')`
  so that users cannot fake native modules by modifying the binding
  object.
- Names the bootstrapping functions so their names show up
  in the stack trace.

PR-URL: nodejs#19112
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
    @joyeecheung
    joyeecheung committed Mar 30, 2018
    Configuration menu
    Copy the full SHA
    ae516be View commit details
    Browse the repository at this point in the history

  2. test: remove NODE_DEBUG in global module loading test 

    Otherwise the debug log output might be mixed up with
the expected errors and the assertion matching the error
message would fail.

PR-URL: nodejs#19177
Refs: nodejs#19112
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
    @joyeecheung
    joyeecheung committed Mar 30, 2018
    Configuration menu
    Copy the full SHA
    d65da3b View commit details
    Browse the repository at this point in the history

  3. src: put bootstrappers in lib/internal/bootstrap/ 

    Create `lib/internal/bootstrap/` and put bootstrappers there:

Before:

```
lib/internal
├── ...
├── bootstrap_loaders.js
└── bootstrap_node.js
```

After:

```
lib/internal
├── ...
└── bootstrap
    ├── loaders.js
    └── node.js
```

PR-URL: nodejs#19177
Refs: nodejs#19112
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
    @joyeecheung
    joyeecheung committed Mar 30, 2018
    Configuration menu
    Copy the full SHA
    3eb7607 View commit details
    Browse the repository at this point in the history

  4. lib: restructure cjs and esm loaders 

    Create `lib/internal/modules` and restructure the module loaders
to make the purpose of those files clearer.

Also make it clear in the code that the object exported by
`lib/internal/modules/cjs/loader.js` is `CJSModule` instead of the
ambiguous `Module`.

Before:

```
lib
├── ...
├── internal
│       ├── loaders
│       │     ├── CreateDynamicModule.js
│       │     ├── DefaultResolve.js
│       │     ├── Loader.js
│       │     ├── ModuleJob.js
│       │     ├── ModuleMap.js
│       │     ├── ModuleWrap.js
│       │     └── Translators.js
│       └── module.js
└── module.js
```

After:

```
lib
├── ...
├── internal
│       ├── ...
│       └── modules
│              ├── cjs
│              │     ├── helpers.js
│              │     └── loader.js
│              └── esm
│                    ├── CreateDynamicModule.js
│                    ├── DefaultResolve.js
│                    ├── Loader.js
│                    ├── ModuleJob.js
│                    ├── ModuleMap.js
│                    └── Translators.js
└── module.js # deleted in this commit to work with git file mode
```

PR-URL: nodejs#19177
Refs: nodejs#19112
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
    @joyeecheung
    joyeecheung committed Mar 30, 2018
    Configuration menu
    Copy the full SHA
    6be0bc5 View commit details
    Browse the repository at this point in the history

  5. lib: add back lib/module.js redirection 

    The previous commit deleted lib/module.js so that git
recognize the file move `lib/module.js` ->
`lib/internal/modules/cjs/loader.js`. This commit add the
redirection back.

PR-URL: nodejs#19177
Refs: nodejs#19112
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
    @joyeecheung
    joyeecheung committed Mar 30, 2018
    Configuration menu
    Copy the full SHA
    f0d9722 View commit details
    Browse the repository at this point in the history