PARSE_ERROR: keep Acorn’s original message somewhere

[rauschma]

Expected Behavior / Situation

Actual Behavior / Situation

Modification Proposal

Context: src/Module.ts

} else if (!module.id.endsWith('.js')) {
  message += ' (Note that you need plugins to import files that are not JavaScript)';
}

• Suggestion for improvement: For error reporting, it would be useful to also have access to Acorn’s original error message, e.g. via a property .parserMessage.
  • FWIW – my particular use case: let users input multiple modules in a web app, bundle them via Rollup, execute the bundle. Then mentioning Rollup is confusing for them. Details: https://2ality.com/2019/10/multi-module-eval.html
• Observation: module.id.endsWith('.js') may need to be changed to also accommodate .mjs.

---

Note by maintainers (by @lukastaegert)

🙋‍♀️ Want to get involved and address this? This issue is likely easy to address and a good starting point to get familiar with the Rollup code base.

The proposed solution is here

rollup/src/Module.ts

Line 133 in 53266e6

} catch (err) {

to attach the original error as an additional parserError property to the object passed to module.error. For that, the RollupError type will need to be extended with an additional optional property.

To test this, a test in the function folder makes sense as those tests are capable of checking error messages, cf. e.g. this test:

rollup/test/function/samples/double-named-export/_config.js

Lines 4 to 21 in 53266e6

	description: 'throws on duplicate named exports',
	error: {
	code: 'PARSE_ERROR',
	message: `Duplicate export 'foo'`,
	pos: 38,
	watchFiles: [path.resolve(__dirname, 'main.js'), path.resolve(__dirname, 'foo.js')],
	loc: {
	file: path.resolve(__dirname, 'foo.js'),
	line: 3,
	column: 9
	},
	frame: `
	1: var foo = 1;
	2: export { foo };
	3: export { foo };
	^
	`
	}

[lukastaegert]

I see no reason why we should not address this, the improvement would be easy and unlikely to break anything. Maybe as a small change, as I am not 100% sold on the name parserMessage, we could add a property .parserError the contains the original error object. This could also be used to debug issues within acorn or from interactions with acorn plugins via the stack trace. As I currently try to get more people involved, I will add some implementation notes to the description for whoever wants to pick this up.

[gribnoysup]

I would love to help with this one 🙌

[lukastaegert]

Nice! As no one else volunteered yet, the issue can be yours. As you probably saw in the description, I added some notes as to what I think needs to be done, just tell me if you need more info. If you are unsure, you can also just start a PR and we can talk about it there. New contributors are always very much welcome!

[lukastaegert]

BTW I actually think you do not need to add a new test, it is probably enough to fix the tests that check for a PARSE_ERROR.

[lukastaegert]

Resolved via #3176