module: improve error decoration for cjs named exports for multi-line import statements

  • Version: v14.11.0
  • Platform: Darwin *** 19.6.0 Darwin Kernel Version 19.6.0: Thu Jun 18 20:49:00 PDT 2020; root:xnu-6153.141.1~1/RELEASE_X86_64 x86_64
  • Subsystem: modules

What steps will reproduce the bug?

Failing test can be found here: ctavan@ba9e734

How often does it reproduce? Is there a required condition?

Always.

What is the expected behavior?

When trying to import named exports from a CommonJS module, there's usually a helpful error message which is assembled here:

if (StringPrototypeIncludes(e.message,
' does not provide an export named')) {
const splitStack = StringPrototypeSplit(e.stack, '\n');
const parentFileUrl = splitStack[0];
const childSpecifier = StringPrototypeMatch(e.message, /module '(.*)' does/)[1];
const childFileURL =
await this.loader.resolve(childSpecifier, parentFileUrl);
const format = await this.loader.getFormat(childFileURL);
if (format === 'commonjs') {
const importStatement = splitStack[1];
const namedImports = StringPrototypeMatch(importStatement, /{.*}/)[0];
const destructuringAssignment = StringPrototypeReplace(namedImports, /\s+as\s+/g, ': ');
e.message = `The requested module '${childSpecifier}' is expected ` +
'to be of type CommonJS, which does not support named exports. ' +
'CommonJS modules can be imported by importing the default ' +
'export.\n' +
'For example:\n' +
`import pkg from '${childSpecifier}';\n` +
`const ${destructuringAssignment} = pkg;`;
const newStack = StringPrototypeSplit(e.stack, '\n');
newStack[3] = `SyntaxError: ${e.message}`;
e.stack = ArrayPrototypeJoin(newStack, '\n');
}

What do you see instead?

When using multi-line import statements like:

import {
  comeOn,
  rightNow,
} from './fail.cjs';

the following regex does not match:

const namedImports = StringPrototypeMatch(importStatement, /{.*}/)[0];

and the following error is produced:

TypeError: Cannot read property '0' of null
      at ModuleJob._instantiate (internal/modules/esm/module_job.js:112:77)
      at async ModuleJob.run (internal/modules/esm/module_job.js:137:5)
      at async Loader.import (internal/modules/esm/loader.js:165:24)
      at async rejects.name (file:///***/node/test/es-module/test-esm-cjs-named-error.mjs:56:3)
      at async waitForActual (assert.js:721:5)
      at async rejects (assert.js:830:25),

Additional information

I would love to contribute a fix for this issue, however I need some guidance on how to proceed. The problem I see is that the full error stack only contains the first line of the multi-line import statement:

[
  'file:///***/node/test/fixtures/es-modules/package-cjs-named-error/multi-line.mjs:2',
  '  comeOn,',
  '  ^^^^^^',
  "SyntaxError: The requested module './fail.cjs' does not provide an export named 'comeOn'",
  '    at ModuleJob._instantiate (internal/modules/esm/module_job.js:98:21)',
  '    at async ModuleJob.run (internal/modules/esm/module_job.js:141:5)',
  '    at async Loader.import (internal/modules/esm/loader.js:165:24)',
  '    at async rejects.name (file:///***/node/test/es-module/test-esm-cjs-named-error.mjs:56:3)',
  '    at async waitForActual (assert.js:721:5)',
  '    at async rejects (assert.js:830:25)'
]

So while the goal of the additional error decoration which was added in #33256 seems to be to provide a copy & pastable solution, I don't see how this could be achieved with the error information at hand when the error comes from a multi-line import statement.

Options that come to my mind:

  • Just resort to the original error?
  • Skip the example if it cannot be generated from the error stack, see: ctavan@248eada
  • Produce something like the following, even though it's incomplete:
      import pkg from './fail.cjs';
      const { comeOn } = pkg;
  • Produce the correct example, which would probably involve using an actual parser to read the full failing import statement.

/cc @MylesBorins