Handling babylon parsing errors in a better way #6961

maazadeeb · 2017-12-03T05:23:13Z

Q	A
Fixed Issues?	Fixes #6205
Patch: Bug Fix?	No
Major: Breaking Change?	No
Minor: New Feature?	No
Tests Added + Pass?	Yes
Documentation PR	No
Any Dependency Changes?	No
License	MIT

Introduces a better way to handle missing plugin errors from babylon, in babel. Previously, the errors used to be directly displayed, as is, from babylon. This was not so friendly as the error used to have only an internal name of the plugin required.

With this change, there is a mapping done between internal plugin names to the external plugin names, where required. There are 2 possible messages that are generated

When a plugin has both syntax and transform plugins:

Support for the experimental syntax [babylon plugin name] isn't currently enabled ([loc]):

[code frame]

Add [npm package name] ([url]) to the 'plugins' section of your Babel config to enable transformation.

When a plugin has only a syntax plugin:

Support for the experimental syntax [babylon plugin name] isn't currently enabled ([loc]):

[code frame]

Add [npm package name] ([url]) to the 'plugins' section of your Babel config to enable parsing.

All the URLs are shortened using git.io, and point directly to the repository field of the plugin's package.json.

maazadeeb · 2017-12-03T05:26:43Z

I need some feedback on writing tests for this. They might probably be fixtures which expect an error to be thrown. If the messages that I have chosen to output are approved, then I'll add tests. As a change in the messages will result in changing the tests.

Also, open to feedback if I should write multiple fixtures or just some sort of unit tests for the new file added.

babel-bot · 2017-12-03T05:31:38Z

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/6213/

nicolo-ribaudo

Thank you! I left a few comments about the error messages; the implementation looks good to me.

You can add the tests in babel-core/test/fixtures. One for each kind of message is enough.

nicolo-ribaudo · 2017-12-03T10:18:12Z

packages/babel-core/src/transformation/normalize-file.js

@@ -102,7 +103,8 @@ function parser(pluginPasses, options, code) {
            },
          },
          options,
-        );
+        ) +
+        `\n${generateHelpMessage(err.missingPlugin)}`;


Can \n be added only if generateHelpMessage doesn't return ""?

Also can we rename generateHelpMessage to something more descriptive like generateMissingPluginMessage?

I'll incorporate both these changes

nicolo-ribaudo · 2017-12-03T10:21:48Z

packages/babel-core/src/transformation/util/missing-plugin-helper.js

+      if (syntaxPlugin) {
+        if (transformPlugin) {
+          helpMessage =
+            `Use the ${syntaxPlugin} plugin to enable parsing. ` +


What about adding This experimental syntax isn't enabled by default as an "inroduction" to the error message?

I'm wondering if we even need a different message in the case where there's only a syntax plugin and not a transform (just dynamic-import)? I feel like we can just simplify this down to something like:

Support for this experimental syntax isn't currently enabled. Add <insert syntax plugin here> to the plugins section of your Babel config.

Support for this experimental syntax isn't currently enabled. Add either <insert transform plugin here> or <insert syntax plugin> (if you only need parsing support) to the plugins section of your Babel config.

I would also add a link to the website because we can explain the user how to install it, and if a transformation is needed. What do you think? That also means we should have doc about all the proposals (or issues if not official)

I'll use @existentialism 's messages. I think they take care of @nicolo-ribaudo 's suggestion as well.
@xtuc like you mentioned, not all the plugins have docs on the website. Would it be better to point to the particular package's README instead?

@maaz93 Yes it is. Since it's a link it's generic enough, we can change it later if needed.

@xtuc Let me clear my understanding. So, if I was suggesting a message for missing asyncGenerators plugin, I would have to add https://github.com/babel/babel/tree/master/packages/babel-plugin-syntax-async-generators and https://github.com/babel/babel/tree/master/packages/babel-plugin-proposal-async-generator-functions in the message? Or I just need to suggest https://babeljs.io/docs/plugins/ for more info?

I can use the READMEs for now, I'm not sure that we have the complete doc on the website tho.

While trying to add this, I realised that the message will get very long with links in it. I thought I'll just suggest

You can find more details about every plugin at https://github.com/babel/babel/tree/master/packages

Will this be enough?

We could even use git.io and make it shorter

@nicolo-ribaudo That was a great suggestion! I wrote a quick dirty script to use gitio in a loop and generate short URLs. The map looks something like this now

asyncGenerators: { syntax: { name: "@babel/plugin-syntax-async-generators", url: "https://git.io/vb4SY", }, transform: { name: "@babel/plugin-proposal-async-generator-functions", url: "https://git.io/vb4yp", }, }

With this, the message would look like

Support for this experimental syntax isn't currently enabled. Add either <insert transform plugin here>(insert URL) or <insert syntax plugin>(insert URL) (if you only need parsing support) to the plugins section of your Babel config.

Is this more clear?

nicolo-ribaudo · 2017-12-03T10:22:51Z

packages/babel-core/src/transformation/util/missing-plugin-helper.js

+        }
+      } else if (issueLink) {
+        helpMessage =
+          `This syntax is not supported yet. ` +


This experimental syntax...

And it's -> its

It this used? That means we are adding early errors in the parser at a very early stage of the proposal, we don't do that currently?

Will add the changes suggested in the first two comments. @xtuc I guess so. I'm not well versed with the parser yet, so I don't have a concrete answer. I just picked up all the plugins from the babylon README.

nicolo-ribaudo · 2017-12-03T10:25:53Z

packages/babel-core/src/transformation/util/missing-plugin-helper.js

+        if (transformPlugin) {
+          helpMessage =
+            `Use the ${syntaxPlugin} plugin to enable parsing. ` +
+            `Use the ${transformPlugin} plugin to enable transformation.` +


Can we suggest the transform plugin before the syntax plugin, since it is the most important one?

xtuc · 2017-12-03T15:52:03Z

Nice first contribution @maaz93, it's so much clearer!

existentialism · 2017-12-11T15:52:18Z

packages/babel-core/src/transformation/normalize-file.js

@@ -103,6 +104,10 @@ function parser(pluginPasses, options, code) {
          },
          options,
        );
+      const missingPluginMessage = generateMissingPluginMessage(missingPlugin);


Just thinking aloud, but looking at this again... right now it outputs something like:

This experimental syntax requires enabling the parser plugin: 'asyncGenerators' (1:14)
> 1 | async function* agf() {
      |                         ^
   2 | await 1;
   3 | yield 2;
   4 | }
Support for this experimental syntax isn't currently enabled. Add either @babel/plugin-syntax-async-generators (https://git.io/vb4SY) or @babel/plugin-syntax-async-generators (https://git.io/vb4SY) (if you only need parsing support) to the 'plugins' section of your Babel config.

Should we just omit the error from Babylon if missingPlugin and maybe just do:

Support for the experimental syntax 'asyncGenerators' isn't currently enabled (1:14)

> 1 | async function* agf() {
      |                         ^
   2 | await 1;
   3 | yield 2;
   4 | }

Add either @babel/plugin-syntax-async-generators (https://git.io/vb4SY) or @babel/plugin-syntax-async-generators (https://git.io/vb4SY) (if you only need parsing support) to the 'plugins' section of your Babel config.

Honestly, I thought about this. But I didn't know how it can be improved. The current message repeats the fact that there is some experimental plugin missing, twice.

I like the new message proposal. It's more meaningful and less repetitive. Can I go ahead and make this change?

The original purpose for making the issue is that yes we would remove the original message and output the new one. I would only add the syntax part for things that we don't have a default transform for, otherwise it should only say the plugin. This is because the use case for babel is the transform not the syntax (because then you would be using babylon by itself)

@hzoo @existentialism Then, the apt message would be

Support for the experimental syntax 'asyncGenerators' isn't currently enabled (1:14)
<codeFrame>
Add @babel/plugin-proposal-async-generator-functions (https://git.io/vb4yp) to the 'plugins' section of your Babel config to enable transformation.

And, in the case where there is only a syntax plugin available

Support for the experimental syntax 'dynamicImport' isn't currently enabled (1:14)
<codeFrame>
Add @babel/plugin-syntax-dynamic-import (https://git.io/vb4Sv) to the 'plugins' section of your Babel config to enable parsing.

Yea, I'm fine with that. Note, can we add line breaks around the codeframe:

Support for the experimental syntax [babylon plugin name] isn't currently enabled ([loc]):

[code frame]

Add [npm package name] ([url]) to the 'plugins' section of your Babel config to enable [parsing|transformation].

nicolo-ribaudo · 2017-12-16T13:03:12Z

(NOTE) If #7008 gets merged before this PR, it should be added to the plugins list.

maazadeeb · 2017-12-17T05:23:22Z

@nicolo-ribaudo I'll do that.

I just had another thought. Do you think it makes sense to write a test case which iterates through all the plugins in the map and makes sure that only syntax or/and transform keys are present per plugin and the url matches https://git.io/* ? I feel this will ensure strict rules on the structure of the map. Only side effect of this would be that I'll have to export the map. Let me know if that would be a good addition to this PR. 😄

existentialism

LGTM.

Seems like adding the tests you mention are slightly overkill, though we can consider stricter enforcement through flow types.

type Plugin = {
  name: string,
  url: string,
};

type PluginNameMap = {
  [name: string]: {
    syntax: Plugin,
    transform?: Plugin,
  },
};

Regardless, I don't consider that a blocker to landing this.

hzoo · 2017-12-18T20:50:08Z

Nice work @maaz93, this will help a lot! Looking forward to how we can improve this as well

maazadeeb · 2017-12-19T08:14:52Z

@hzoo Thanks! I would love to improve it based on any suggestions or help anyone who's looking to make any improvements, if needed. 😄

Handling babylon parsing errors in a better way

7fe1fac

nicolo-ribaudo reviewed Dec 3, 2017

View reviewed changes

nicolo-ribaudo added the PR: Polish 💅 A type of pull request used for our changelog categories label Dec 3, 2017

xtuc added the pkg: parser label Dec 3, 2017

existentialism mentioned this pull request Dec 4, 2017

Using babel 7 with env preset: object spread causes error about missing objectRestSpread parser plugin #6970

Closed

Better error messages + Helpful URLs

82bf668

existentialism reviewed Dec 11, 2017

View reviewed changes

Replaced message from babylon completely

e51b174

Add importMeta plugin to the map

4beb4df

existentialism approved these changes Dec 18, 2017

View reviewed changes

hzoo merged commit 17b37b5 into babel:master Dec 18, 2017

maazadeeb deleted the better-errors branch January 14, 2018 16:50

lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Oct 5, 2019

lock bot locked as resolved and limited conversation to collaborators Oct 5, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Handling babylon parsing errors in a better way #6961

Handling babylon parsing errors in a better way #6961

maazadeeb commented Dec 3, 2017 •

edited

maazadeeb commented Dec 3, 2017

babel-bot commented Dec 3, 2017 •

edited

nicolo-ribaudo left a comment •

edited

nicolo-ribaudo Dec 3, 2017

existentialism Dec 3, 2017 •

edited

maazadeeb Dec 5, 2017

nicolo-ribaudo Dec 3, 2017

existentialism Dec 3, 2017 •

edited

xtuc Dec 3, 2017

maazadeeb Dec 5, 2017

xtuc Dec 5, 2017

maazadeeb Dec 8, 2017 •

edited

xtuc Dec 8, 2017

maazadeeb Dec 8, 2017

nicolo-ribaudo Dec 8, 2017

maazadeeb Dec 9, 2017 •

edited

nicolo-ribaudo Dec 3, 2017

existentialism Dec 3, 2017

xtuc Dec 3, 2017

maazadeeb Dec 5, 2017

nicolo-ribaudo Dec 3, 2017

maazadeeb Dec 5, 2017

xtuc commented Dec 3, 2017

existentialism Dec 11, 2017

maazadeeb Dec 11, 2017

hzoo Dec 11, 2017

maazadeeb Dec 12, 2017 •

edited

existentialism Dec 12, 2017

nicolo-ribaudo commented Dec 16, 2017

maazadeeb commented Dec 17, 2017

existentialism left a comment

hzoo commented Dec 18, 2017

maazadeeb commented Dec 19, 2017

Handling babylon parsing errors in a better way #6961

Handling babylon parsing errors in a better way #6961

Conversation

maazadeeb commented Dec 3, 2017 • edited

maazadeeb commented Dec 3, 2017

babel-bot commented Dec 3, 2017 • edited

nicolo-ribaudo left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

existentialism Dec 3, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

existentialism Dec 3, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maazadeeb Dec 8, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maazadeeb Dec 9, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xtuc commented Dec 3, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maazadeeb Dec 12, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nicolo-ribaudo commented Dec 16, 2017

maazadeeb commented Dec 17, 2017

existentialism left a comment

Choose a reason for hiding this comment

hzoo commented Dec 18, 2017

maazadeeb commented Dec 19, 2017

maazadeeb commented Dec 3, 2017 •

edited

babel-bot commented Dec 3, 2017 •

edited

nicolo-ribaudo left a comment •

edited

existentialism Dec 3, 2017 •

edited

existentialism Dec 3, 2017 •

edited

maazadeeb Dec 8, 2017 •

edited

maazadeeb Dec 9, 2017 •

edited

maazadeeb Dec 12, 2017 •

edited