- 
          
- 
                Notifications
    You must be signed in to change notification settings 
- Fork 33.6k
module: make CJS load from ESM loader #47999
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
Changes from all commits
3deeccf
              c1b346c
              df14ab8
              f5fdea1
              598d789
              afd882a
              9958f1f
              6d57350
              ad7a8ed
              d0e4abf
              39219be
              739dc9e
              297a63b
              32bf878
              74c93d1
              0a63101
              a6f01a3
              3b39ba3
              dd2af79
              f692715
              850831b
              c7edb23
              2a5521e
              507cd21
              ec6d178
              279e19d
              454985b
              2c499bc
              2c5fcfb
              8291086
              624d933
              5d9cdd0
              File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|  | @@ -10,6 +10,7 @@ const { kEmptyObject } = require('internal/util'); | |||||||||||||||||||||||||||||||||||
| const { defaultGetFormat } = require('internal/modules/esm/get_format'); | ||||||||||||||||||||||||||||||||||||
| const { validateAssertions } = require('internal/modules/esm/assert'); | ||||||||||||||||||||||||||||||||||||
| const { getOptionValue } = require('internal/options'); | ||||||||||||||||||||||||||||||||||||
| const { readFileSync } = require('fs'); | ||||||||||||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||||||||||||
| // Do not eagerly grab .manifest, it may be in TDZ | ||||||||||||||||||||||||||||||||||||
| const policy = getOptionValue('--experimental-policy') ? | ||||||||||||||||||||||||||||||||||||
|  | @@ -69,12 +70,35 @@ async function getSource(url, context) { | |||||||||||||||||||||||||||||||||||
| return { __proto__: null, responseURL, source }; | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||||||||||||
| function getSourceSync(url, context) { | ||||||||||||||||||||||||||||||||||||
| const parsed = new URL(url); | ||||||||||||||||||||||||||||||||||||
| const responseURL = url; | ||||||||||||||||||||||||||||||||||||
| let source; | ||||||||||||||||||||||||||||||||||||
| if (parsed.protocol === 'file:') { | ||||||||||||||||||||||||||||||||||||
| source = readFileSync(parsed); | ||||||||||||||||||||||||||||||||||||
| } else if (parsed.protocol === 'data:') { | ||||||||||||||||||||||||||||||||||||
| const match = RegExpPrototypeExec(DATA_URL_PATTERN, parsed.pathname); | ||||||||||||||||||||||||||||||||||||
|         
                  GeoffreyBooth marked this conversation as resolved.
              Show resolved
            Hide resolved | ||||||||||||||||||||||||||||||||||||
| if (!match) { | ||||||||||||||||||||||||||||||||||||
| throw new ERR_INVALID_URL(url); | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| const { 1: base64, 2: body } = match; | ||||||||||||||||||||||||||||||||||||
| source = BufferFrom(decodeURIComponent(body), base64 ? 'base64' : 'utf8'); | ||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||
| const supportedSchemes = ['file', 'data']; | ||||||||||||||||||||||||||||||||||||
|         
                  aduh95 marked this conversation as resolved.
              Show resolved
            Hide resolved | ||||||||||||||||||||||||||||||||||||
| throw new ERR_UNSUPPORTED_ESM_URL_SCHEME(parsed, supportedSchemes); | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| if (policy?.manifest) { | ||||||||||||||||||||||||||||||||||||
| policy.manifest.assertIntegrity(parsed, source); | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| 
      Comment on lines
    
      +90
     to 
      +92
    
   There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 
        Suggested change
       
 (possibly might need another  There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I copied that from  | ||||||||||||||||||||||||||||||||||||
| return { __proto__: null, responseURL, source }; | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||
| * Node.js default load hook. | ||||||||||||||||||||||||||||||||||||
| * @param {string} url | ||||||||||||||||||||||||||||||||||||
| * @param {object} context | ||||||||||||||||||||||||||||||||||||
| * @returns {object} | ||||||||||||||||||||||||||||||||||||
| * @param {LoadContext} context | ||||||||||||||||||||||||||||||||||||
| * @returns {LoadReturn} | ||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||
| async function defaultLoad(url, context = kEmptyObject) { | ||||||||||||||||||||||||||||||||||||
| let responseURL = url; | ||||||||||||||||||||||||||||||||||||
|  | @@ -108,6 +132,51 @@ async function defaultLoad(url, context = kEmptyObject) { | |||||||||||||||||||||||||||||||||||
| source, | ||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||
| * @typedef LoadContext | ||||||||||||||||||||||||||||||||||||
| * @property {string} [format] A hint (possibly returned from `resolve`) | ||||||||||||||||||||||||||||||||||||
| * @property {string | Buffer | ArrayBuffer} [source] source | ||||||||||||||||||||||||||||||||||||
| * @property {Record<string, string>} [importAssertions] import attributes | ||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||
| * @typedef LoadReturn | ||||||||||||||||||||||||||||||||||||
| * @property {string} format format | ||||||||||||||||||||||||||||||||||||
| * @property {URL['href']} responseURL The module's fully resolved URL | ||||||||||||||||||||||||||||||||||||
| * @property {Buffer} source source | ||||||||||||||||||||||||||||||||||||
|         
                  JakobJingleheimer marked this conversation as resolved.
              Show resolved
            Hide resolved | ||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||
| * @param {URL['href']} url | ||||||||||||||||||||||||||||||||||||
| * @param {LoadContext} [context] | ||||||||||||||||||||||||||||||||||||
| * @returns {LoadReturn} | ||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||
| function defaultLoadSync(url, context = kEmptyObject) { | ||||||||||||||||||||||||||||||||||||
| let responseURL = url; | ||||||||||||||||||||||||||||||||||||
| const { importAssertions } = context; | ||||||||||||||||||||||||||||||||||||
| let { | ||||||||||||||||||||||||||||||||||||
| format, | ||||||||||||||||||||||||||||||||||||
| source, | ||||||||||||||||||||||||||||||||||||
| } = context; | ||||||||||||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||||||||||||
| format ??= defaultGetFormat(new URL(url), context); | ||||||||||||||||||||||||||||||||||||
| 
      Comment on lines
    
      +156
     to 
      +162
    
   There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Format does not / should not change, it just needs a default value, so I think it would be better to keep it a constant with a default: 
        Suggested change
       
 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What if  There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, we have a bug in the  node/lib/internal/modules/esm/hooks.js Lines 336 to 339 in d150316 
 
 Let's fix them together (we can't fix this part without creating a second bug). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm pretty sure we've always accepted nullish values indifferently in the hook APIs. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Even for enums? Why?  | ||||||||||||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||||||||||||
| validateAssertions(url, format, importAssertions); | ||||||||||||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||||||||||||
| if (format === 'builtin') { | ||||||||||||||||||||||||||||||||||||
| source = null; | ||||||||||||||||||||||||||||||||||||
| } else if (source == null) { | ||||||||||||||||||||||||||||||||||||
| ({ responseURL, source } = getSourceSync(url, context)); | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||||||||||||
| return { | ||||||||||||||||||||||||||||||||||||
| __proto__: null, | ||||||||||||||||||||||||||||||||||||
| format, | ||||||||||||||||||||||||||||||||||||
| responseURL, | ||||||||||||||||||||||||||||||||||||
| source, | ||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||
| * throws an error if the protocol is not one of the protocols | ||||||||||||||||||||||||||||||||||||
|  | @@ -160,5 +229,6 @@ function throwUnknownModuleFormat(url, format) { | |||||||||||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||||||||||||
| module.exports = { | ||||||||||||||||||||||||||||||||||||
| defaultLoad, | ||||||||||||||||||||||||||||||||||||
| defaultLoadSync, | ||||||||||||||||||||||||||||||||||||
| throwUnknownModuleFormat, | ||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.