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

[ctavan]

• 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:

node/lib/internal/modules/esm/module_job.js

Lines 102 to 124 in ed8af4e

	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:

node/lib/internal/modules/esm/module_job.js

Line 112 in ed8af4e

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

[ljharb]

cc @nodejs/modules-active-members

[MylesBorins]

This is definitely less than ideal. I like the solution in ctavan@248eada as a place to start. We can definitely improve the regex and support multi-line in the future.

[ctavan]

Great, I‘ll provide a PR tomorrow.