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

Jump to bottom

fix: handle process[toString] being readonly on newer nodes #8050

Merged
merged 4 commits into from Mar 5, 2019
Merged

fix: handle process[toString] being readonly on newer nodes #8050

merged 4 commits into from Mar 5, 2019

Conversation

SimenB
Copy link
Member

@SimenB SimenB commented Mar 5, 2019

Summary

If we do not do this, Jest fails horribly on Node 12 (tested with yesterday's nightly release)

image

Beyond this failure, the only things that fails are the leak detector tests since weak does not compile on Node 12

Test plan

Green CI (although we don't test node 12 yet, at least avoid regressions)

@SimenB SimenB requested a review from mjesun March 5, 2019 10:12
@SimenB SimenB mentioned this pull request Mar 5, 2019
Closed
@SimenB SimenB changed the title fix: handle process[toString] being readonly on enwer nodes fix: handle process[toString] being readonly on newer nodes Mar 5, 2019
@rubennorte rubennorte self-requested a review March 5, 2019 12:22
rubennorte
rubennorte approved these changes Mar 5, 2019
View reviewed changes
Copy link
Contributor

@rubennorte rubennorte left a comment

Choose a reason for hiding this comment

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

Looks good! It seems that we won't need additional tests when we add Node 12 to the build, as otherwise all tests would fail horribly, right?

@SimenB
Copy link
Member Author

SimenB commented Mar 5, 2019

yeah

@SimenB SimenB merged commit f526a91 into jestjs:master Mar 5, 2019
@SimenB SimenB deleted the create-process-node-12 branch March 5, 2019 12:33
@msand
Copy link

msand commented Mar 6, 2019

This started failing on latest node with the https://nodejs.org/en/blog/release/v11.11.0/ release an hour ago. Getting this error:
TypeError: Cannot assign to read only property 'Symbol(Symbol.toStringTag)' of object '#<process>'

https://travis-ci.org/msand/react-native-svg-e2e/jobs/502745672

Should v24.2.0-alpha.0 be used for now?

msand added a commit to msand/react-native-svg-e2e that referenced this pull request Mar 6, 2019
@msand
Fix jestjs/jest#8050
6c7e597 
TypeError: Cannot assign to read only property 'Symbol(Symbol.toStringTag)' of object '#<process>'

https://travis-ci.org/msand/react-native-svg-e2e/jobs/502745672
msand added a commit to msand/react-native-svg-e2e that referenced this pull request Mar 6, 2019
@msand
Use latest stable yarn to make resolutions work
73575ab 
Fix jestjs/jest#8050

TypeError: Cannot assign to read only property 'Symbol(Symbol.toStringTag)' of object '#<process>'

https://travis-ci.org/msand/react-native-svg-e2e/jobs/502745672
@jeysal jeysal mentioned this pull request Mar 6, 2019
Closed
@danielnass
Copy link

@msand I'm getting this error too

This was referenced Mar 7, 2019
Closed
Closed
@SimenB
Copy link
Member Author

SimenB commented Mar 7, 2019

☹️

You can install jest@beta for now which includes this fix

@SimenB SimenB mentioned this pull request Mar 7, 2019
Closed
@mattzeunert mattzeunert mentioned this pull request Mar 7, 2019
Merged
@snags88 snags88 mentioned this pull request Mar 7, 2019
Merged
@macklinu macklinu mentioned this pull request Mar 8, 2019
Merged
2 tasks
IJMacD added a commit to IJMacD/query that referenced this pull request Mar 8, 2019
@IJMacD
Pin ci node version due to jestjs/jest#8050
4cdd4bd
ascartabelli added a commit to ascartabelli/lamb that referenced this pull request Mar 8, 2019
@ascartabelli
- Updated jest-cli to avoid errors with Node >= 11.11.0 (see jestjs/j…
e4fece6 
…est#8050)

- Updated eslint rules
- Removed `gulp-jest`
pascalw added a commit to pascalw/dashbling that referenced this pull request Mar 10, 2019
@pascalw
Upgrade Jest
f533ae5 
This resolves an issue on Node 11.x and up.

See https://stackoverflow.com/questions/55059748/travis-jest-typeerror-cannot-assign-to-read-only-property-symbolsymbol-tostr
and jestjs/jest#8050 for details.
@pascalw pascalw mentioned this pull request Mar 10, 2019
Merged
@micalgenus micalgenus mentioned this pull request Mar 11, 2019
Closed
ascartabelli added a commit to ascartabelli/lamb that referenced this pull request Mar 11, 2019
@ascartabelli
- Updated jest-cli to avoid errors with Node >= 11.11.0 (see jestjs/j…
9470b96 
…est#8050)

- Updated eslint rules
@ZauberNerd
Copy link

@SimenB would it be possible to also release a fix for Jest 23? Or is that discontinued now that v24 is out?

@SimenB
Copy link
Member Author

SimenB commented Mar 11, 2019

Yes, please upgrade to 24. Alternatively, you can probably use a resolution for jest-util if needed.

Why are you blocked from upgrading to 24?

@ZauberNerd
Copy link

@SimenB thanks for your quick response!
We have built a tool (similar to CRA or Next.js) to allow our teams to easily spin up new projects, but if we were to upgrade to Jest 24 that would be a breaking change for us (at least at a first glance - I haven't had the time yet to look deeper into this) and therefore I have currently put the upgrade on hold (https://github.com/xing/hops/pulls).
But I will try to find some time this week to look into it to see how we could upgrade to Jest 24 ;)

@SimenB
Copy link
Member Author

SimenB commented Mar 11, 2019 •
edited

The breaking changes should be clearly noted as such in the changelog. And they should be relatively minor. Hopefully you'll have an easy time of it! 🤞

@manniL manniL mentioned this pull request Mar 12, 2019
Merged
1 task
@cdrani cdrani mentioned this pull request Apr 6, 2019
Closed
@snyk-bot snyk-bot mentioned this pull request Dec 15, 2019
Open
This was referenced Mar 21, 2020
Closed
Merged
@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 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@rubennorte rubennorte rubennorte approved these changes

@mjesun mjesun Awaiting requested review from mjesun

Assignees
No one assigned
Labels
cla signed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@SimenB @msand @danielnass @ZauberNerd @rubennorte @facebook-github-bot