Skip to content
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

Add descriptive message for 'unexpected token' errors #6275

Merged
merged 7 commits into from
May 26, 2018

Conversation

thymikee
Copy link
Collaborator

@thymikee thymikee commented May 26, 2018

Summary

We had a lot of issues with "unexpected token" errors. Time to actually throw something more descriptive :)

Attaching a screenshot (the part above /Users/... is added with this PR):
screen shot 2018-05-26 at 15 33 51

Pretty please check the language if we can make the message easier to comprehend 🙏

Test plan

Integration test

expect(stdout).toBe('');
expect(stderr).toMatch(/Jest encountered an unexpected token/);
expect(stderr).toMatch(/import {module}/);
expect(stderr).toMatch(/Unexpected token import/);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would it make sense to exatrct the whole thing and snapshot it? Just to make sure formatting doesn't get messed up accidentally

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we're good with not snapshotting it, copy and indentation changes will be clearly visible in the PR diff

@@ -315,6 +316,36 @@ export default class ScriptTransformer {
e.stack = e.codeFrame;
}

if (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we also take it outside of this module and make it more declarative? it makes it really bloated :)
like

if (
  e instanceof SyntaxError &&
  e.message.includes('Unexpected token') &&
  !e.message.includes(' expected')
) {
  e = addTransformErrorMessage(e);
}

@thymikee
Copy link
Collaborator Author

cc @captbaritone

SimenB
SimenB previously approved these changes May 26, 2018
@SimenB SimenB dismissed their stale review May 26, 2018 13:32

Wait no, CI failed on node 10 on your new test

e.stack =
`${chalk.bold.red('Jest encountered an unexpected token')}

This usually means that you are trying to import a file which Jest cannot parse, e.g. it's not JavaScript.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be more accurate to say this you are trying to import a file which node cannot parse?


If you need to read non-JS files (e.g. binary assets), stubbing them out with ${chalk.bold(
'"moduleNameMapper"',
)} should help.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about:

Jest encountered an unexpected token

This usually means that you are trying to import a file which Jest cannot parse, e.g. it's not plain JavaScript.

By default, if Jest sees a .babelrc, it will use that to transform your files, ignoring node_modules. To have some, of your node_modules files transformed, you can specify a custom "transformIgnorePatterns" in your config.

If you need a custom transformation specify a "transform" option in your config.

If you simply want to mock your non-JS modules (e.g. binary assets) you can stub them out with the "moduleNameMapper" config option.

@thymikee
Copy link
Collaborator Author

Updated the test and the copy

@thymikee thymikee merged commit 5c6007f into jestjs:master May 26, 2018
@thymikee thymikee deleted the feat/unexpected-token-error branch May 26, 2018 15:09
@SimenB SimenB mentioned this pull request Aug 13, 2018
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants