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

fix(template): Fix jest bugs in react template #881

Closed
wants to merge 1 commit into from
Closed

fix(template): Fix jest bugs in react template #881

wants to merge 1 commit into from

Conversation

FDiskas
Copy link
Contributor

@FDiskas FDiskas commented May 15, 2019

This pull request closes issue # (put the issue number here)

Current state

npm init hops-app my-hops-app
cd my-hops-app
npm test

Requires Babel "^7.0.0-0", but was loaded with "6.26.3". If you are sure you have a compatible version of @babel/core, it is likely that something in your build process is loading th
e wrong version. Inspect the stack trace of this error to look for the first entry that doesn't mention "@babel/core" or "babel-core" to see what is calling Babel.

Changes introduced here

How to fix:
npx babel-upgrade --write --install
npm audit fix --force

NOTE And commit pakage-lock.json or yarn.lock to git for keeping versions locked and not introduce unexpected errors for end users

Checklist

  • All commit messages adhere to the appropriate format in order to trigger a correct release
  • All code is written in untranspiled ECMAScript (ES 2017) and is formatted using prettier
  • Necessary unit tests are added in order to ensure correct behavior
  • Documentation has been added

Copy link
Contributor

@ZauberNerd ZauberNerd left a comment

Choose a reason for hiding this comment

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

Also on another note: Could you please apply this fix to hops-template-graphql and hops-template-redux as well?
And, while you are at it, could you please reword the commit message to something like: fix(template-react): add babel-core as dev dependency (and fix(template-graphql): and fix(template-redux): for the others)?

@@ -25,7 +25,9 @@
"react-router-dom": "^5.0.0"
},
"devDependencies": {
"jest": "^23.4.2",
"babel-core": "^7.0.0-bridge.0",
"babel-jest": "^24.8.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

I would prefer to remove babel-jest here, because we have it already as a dependency in the jest-preset-hops:
https://github.com/xing/hops/blob/master/packages/jest-preset/package.json#L31

Also, I would like to keep jest at version 23 for now, because unfortunately we still have some discussion points open, for how we could upgrade to jest 24 without being breaking for our users (see #815 and https://github.com/xing/hops/pulls?q=is%3Apr+is%3Aopen+label%3Ablocked)

I will try to raise this topic to our attention and investigate if we can find a solution where we can safely upgrade to jest 24, but until then we should stay at 23 to be on the safe side.

So in a nutshell: It will be enough to just add babel-core@7.0.0-bridge.0 to the dev dependencies ;)

@ZauberNerd
Copy link
Contributor

Hi @FDiskas do you want to continue on this PR or should we take over and implement this ourselves?

@FDiskas
Copy link
Contributor Author

FDiskas commented May 27, 2019

Yes please. I'm gone for holiday

@ZauberNerd
Copy link
Contributor

@FDiskas I continued your PR here: #887

@ZauberNerd ZauberNerd closed this May 28, 2019
@FDiskas FDiskas deleted the bugfix/react-app-template-jest-fox branch July 4, 2019 19:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants