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

docs: setting up babel, fixes regeneratorRuntime not defined #7701

Merged
merged 11 commits into from
Jan 26, 2019

Conversation

DylanVann
Copy link
Contributor

@DylanVann DylanVann commented Jan 25, 2019

Summary

If a user follows the current instructions for setting up Babel they will not be able to use async/await, and in fact a cryptic error will be thrown.

The error is: ReferenceError: regeneratorRuntime is not defined

See: #7579

This PR updates the instructions to include using @babel/polyfill so that this error does not occur.
This PR updates the instructions to include configuring @babel/preset-env's targets option.

Test plan

I have followed the new instructions to install Jest and setup Babel, async/await works correctly.

Copy link
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

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

Thanks!

Left a couple of inline comments, but I don't agree with the direction here.

Could we instead have something at the bottom (below our example config file) saying something like

Remember to correctly configure @babel/preset-env for your transpilation targets. See Babel's docs.

Not sure if something like this makes sense as well?

Jest will set process.env.NODE_ENV to 'test' if it's not set to something else, so you can use that in your configuration to conditionally setup only the transpilation you need, e.g. adding {targets: {node: 'current'}}

docs/GettingStarted.md Outdated Show resolved Hide resolved
docs/GettingStarted.md Outdated Show resolved Hide resolved
docs/GettingStarted.md Outdated Show resolved Hide resolved
@DylanVann
Copy link
Contributor Author

I had to remove "You are now set up to use all ES2018 features and React-specific syntax.". If you're running an older Node version certain things might not be available without using the polyfill, like String.prototype.trimStart().


```bash
yarn add --dev babel-jest @babel/core
yarn add --dev babel-jest @babel/core @babel/preset-env @babel/preset-react
Copy link
Member

Choose a reason for hiding this comment

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

you don't need either of the presets - the other 2 are actually needed, though. Not sure how to best say that, and still have useful examples

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well you'd probably want to include something, at the very least you probably want to compile ES6 modules.

I think our bases are pretty well covered by "The ideal configuration for Babel will depend on your project.".

Copy link
Collaborator

Choose a reason for hiding this comment

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

We assume user already have Babel set up in the project and they only want to add Jest support, hence only babel-jest and @babel/core are needed. They need to be at top level of node_modules to satisfy peerDependencies and transform resolver

Copy link
Member

Choose a reason for hiding this comment

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

for babel 7, people are required to use @babel/core. So if we go with the assumption babel is correctly set up, we just need to add babel-jest

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it's sane to assume that Babel is there and we should make it obvious in the guide. We can't choose Babel plugins and presets for users, that's bad.

So, only adding babel-jest should suffice. We just need to make sure users have Babel 7 already set up.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well I do think no instructions would be better than not working instructions 😆.

If it was up to me I'd remove the React preset from these instructions, but otherwise leave them as is.

docs/GettingStarted.md Outdated Show resolved Hide resolved
docs/GettingStarted.md Outdated Show resolved Hide resolved
docs/GettingStarted.md Show resolved Hide resolved
@SimenB
Copy link
Member

SimenB commented Jan 25, 2019

@DylanVann now that we've released Jest 24, you need to also update the versioned docs: https://github.com/facebook/jest/blob/master/website/versioned_docs/version-24.0/GettingStarted.md

EDIT: oops, wrong tag 😬

@DylanVann
Copy link
Contributor Author

@SimenB Do we agree on the content? I've put in bold:

The ideal configuration for Babel will depend on your project.

Copy link
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

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

lgtm, thanks! Just merge in master an add it to versioned docs and we can merge this

CHANGELOG.md Outdated
@@ -171,6 +171,7 @@
- `[diff-sequences]` Add performance benchmark to package ([#7603](https://github.com/facebook/jest/pull/7603))
- `[*]` Replace as many `Object.assign` with object spread as possible ([#7627](https://github.com/facebook/jest/pull/7627))
- `[ci]` Initial support for Azure Pipelines ([#7556](https://github.com/facebook/jest/pull/7556))
- `[docs]` Changed Babel setup documentation to fix `async/await` ([#7701](https://github.com/facebook/jest/pull/7701))
Copy link
Member

Choose a reason for hiding this comment

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

After merging in master, please move up to correct section

@SimenB
Copy link
Member

SimenB commented Jan 26, 2019

Do we agree on the content?

Yeah, although the basis (using babel.config.js, installing babel-jest) are always needed. Beyond that, it really depends, though 🙂

@DylanVann
Copy link
Contributor Author

@SimenB Done, thanks for the help 🙂

Copy link
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

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

This is great, thank you!

@SimenB SimenB merged commit 32bb032 into jestjs:master Jan 26, 2019
captain-yossarian pushed a commit to captain-yossarian/jest that referenced this pull request Jul 18, 2019
@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.

4 participants