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 missing imports in react projects #43

Merged
merged 5 commits into from Oct 31, 2020
Merged

Conversation

joebochill
Copy link
Collaborator

@joebochill joebochill commented Oct 27, 2020

The latest version of create-react-app no longer adds a line for importing the service worker. We were using this in our find/replace script to add our imports into the index. So projects started with the CLI are missing some required imports in order to run.

Changes proposed in this Pull Request:

  • key off of the ReactDom.render line instead of the preceding import line to add our imports
  • update app template to pass linting checks

@jeffvg
Copy link
Contributor

jeffvg commented Oct 27, 2020

just looking at this...

The react-scripts package provided by Create React App requires a dependency:

  • "eslint": "^7.11.0"

currently package includes

  • "eslint": "6.6.0"

plus
"eslint-config-prettier": "6.13.0",
"eslint-plugin-react": "7.0.0",

If I upgrade eslint to 7.11.0 at compile get this error. seems react-scripts is set to 4.0.0 when is might need to be 3.4.3?

image

@joebochill
Copy link
Collaborator Author

Looks like react 17, create react app 4, and react-scripts 4 have some undesirable behavior.

facebook/create-react-app#9887

@joebochill
Copy link
Collaborator Author

create-react-app pulls the latest version of react and react-scripts, so we can't even lock down on the older version of CRA in our CLI to ride this out.

The other option is to try to fix these linting errors in the generated files programatically.

@joebochill
Copy link
Collaborator Author

I updated the App.tsx file to pass our linting rules. We will need to publish a new version of the lint config to get the last couple errors: etn-ccis/blui-code-standards#29

Copy link
Contributor

@emclaug2 emclaug2 left a comment

Choose a reason for hiding this comment

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

I'm seeing this error which is strange because ^7.0.0 is the specified eslint version to use.

This happens when I run yarn start. Can you double-check this is working on your end?

image

@joebochill
Copy link
Collaborator Author

@emclaug2 do you have the cli linked up correctly to use the local version? If you do yarn why eslint, what does it say?

@joebochill
Copy link
Collaborator Author

joebochill commented Oct 28, 2020

I explicitly made the minimum eslint version 7.11.0 even though ^7.0.0 should have picked that up anyway, just to be safe. The newly published eslint-config should also clear up any lint errors.

A new project should spin up nicely without issue now.

@jeffvg
Copy link
Contributor

jeffvg commented Oct 28, 2020

I'm seeing this now, strange. The default react app displays for a split second then kicks this out

image

@joebochill
Copy link
Collaborator Author

Did you pull the latest CLI code and start a fresh project? What settings did you use in the CLI?

@jeffvg
Copy link
Contributor

jeffvg commented Oct 28, 2020

Did you pull the latest CLI code and start a fresh project? What settings did you use in the CLI?

yeah, just pulled latest, yarn and ran pxb new. > react and yes to everything

@jeffvg
Copy link
Contributor

jeffvg commented Oct 28, 2020

did unlink and link on my PC to see what happens. I'm good on mac

image

yarn unlink and linking back didn't change anything. Did a fresh clone and started over. Ended up with same results as before.

image

@jeffvg
Copy link
Contributor

jeffvg commented Oct 28, 2020

If I down grade these to 4.0.1 in my new react project it runs fine

  • "@typescript-eslint/eslint-plugin": "4.0.1",
  • "@typescript-eslint/parser": "4.0.1"

@joebochill
Copy link
Collaborator Author

@jeffvg you said mac is fine? Do you have the cli installed globally on your PC? Global installations can cause problems with local yarn link.

@jeffvg
Copy link
Contributor

jeffvg commented Oct 28, 2020

@jeffvg you said mac is fine? Do you have the cli installed globally on your PC? Global installations can cause problems with local yarn link.

I have no issues on my mac. Only PC.
I don't have a global install anymore since we started using "npx"

C:\pxblue
λ pxb new
'pxb' is not recognized as an internal or external command,
operable program or batch file.

@joebochill
Copy link
Collaborator Author

@jeffvg I've published a beta package to NPM to see if that gets around the linking issues. You can try:

npx -p @pxblue/cli@beta pxb new react

@jeffvg
Copy link
Contributor

jeffvg commented Oct 29, 2020

@jeffvg I've published a beta package to NPM to see if that gets around the linking issues. You can try:

npx -p @pxblue/cli@beta pxb new react

the beta worked out the same for me with same errors as before.
Changing these packages from 4.5.0 to 4.0.1 then I can run.
"@typescript-eslint/eslint-plugin": "4.0.1",
"@typescript-eslint/parser": "4.0.1",

@emclaug2 emclaug2 self-requested a review October 29, 2020 14:46
@emclaug2
Copy link
Contributor

This is so strange; I got the same error as @jeffvg 👍
image

This error should be caused by an older version of our eslint config rules.

Even though the specified version of the @pxblue/eslint-config is ^2.0.2, I did not receive the latest package in the generated project:

image

@joebochill
Copy link
Collaborator Author

joebochill commented Oct 29, 2020

That's really strange. You're not getting the ^s at all. You are installing absolute versions.

Can you go into the generated project and run yarn add lodash@^4.0.0 and see what gets added to your package.json?

yarnpkg/yarn#3270

@jeffvg
Copy link
Contributor

jeffvg commented Oct 29, 2020

That's really strange. You're not getting the ^s at all. You are installing absolute versions.

Can you go into the generated project and run yarn add lodash@^4.0.0 and see what gets added to your package.json?

yarnpkg/yarn#3270

same as issue you linked to, no caret ^

image

@jeffvg
Copy link
Contributor

jeffvg commented Oct 29, 2020

anyone use "yarn check"? Its been a while for me but here is some output from it on a new React project.

error "@material-ui/core##react@^16.8.0" doesn't satisfy found match of "react@17.0.1"
error "@material-ui/core##react-dom@^16.8.0" doesn't satisfy found match of "react-dom@17.0.1"
error "@material-ui/icons##react@^16.8.0" doesn't satisfy found match of "react@17.0.1"
error "@material-ui/icons##react-dom@^16.8.0" doesn't satisfy found match of "react-dom@17.0.1"
error "@pxblue/react-components##react@^16.8.6" doesn't satisfy found match of "react@17.0.1"
error "@pxblue/react-components##react-dom@^16.8.6" doesn't satisfy found match of "react-dom@17.0.1"
error "react-scripts#typescript@^3.2.1" doesn't satisfy found match of "typescript@4.0.5"
error "@pxblue/eslint-config##@typescript-eslint/eslint-plugin@^3.0.0" doesn't satisfy found match of "@typescript-eslint\eslint-plugin@4.5.0"
error "enzyme-adapter-react-16#react@^16.0.0-0" doesn't satisfy found match of "react@17.0.1"
error "enzyme-adapter-react-16#react-dom@^16.0.0-0" doesn't satisfy found match of "react-dom@17.0.1"
error "eslint-plugin-react#eslint@^3.0.0" doesn't satisfy found match of "eslint@7.11.0"
error "@material-ui/core#@material-ui/styles##react@^16.8.0" doesn't satisfy found match of "react@17.0.1"
error "@material-ui/core#@material-ui/styles##react-dom@^16.8.0" doesn't satisfy found match of "react-dom@17.0.1"
error "@material-ui/core#@material-ui/system##react@^16.8.0" doesn't satisfy found match of "react@17.0.1"
error "@material-ui/core#@material-ui/system##react-dom@^16.8.0" doesn't satisfy found match of "react-dom@17.0.1"
error "@material-ui/core#@material-ui/utils##react@^16.8.0" doesn't satisfy found match of "react@17.0.1"
error "@material-ui/core#@material-ui/utils##react-dom@^16.8.0" doesn't satisfy found match of "react-dom@17.0.1"
error "react-scripts#@pmmmwh/react-refresh-webpack-plugin##type-fest@^0.13.1" doesn't satisfy found match of "type-fest@0.8.1"
error "enzyme-adapter-react-16#enzyme-adapter-utils#react@0.13.x || 0.14.x || ^15.0.0-0 || ^16.0.0-0" doesn't satisfy found match of "react@17.0.1"
warning "jest-resolve#jest-pnp-resolver#jest-resolve@*" could be deduped from "26.6.1" to "jest-resolve@26.6.1"
error "enzyme-adapter-react-16#react-test-renderer#react@^16.14.0" doesn't satisfy found match of "react@17.0.1"
error "enzyme-adapter-utils#airbnb-prop-types#react@^0.14 || ^15.0.0 || ^16.0.0-alpha" doesn't satisfy found match of "react@17.0.1"
error Found 21 errors.

@huayunh
Copy link
Contributor

huayunh commented Oct 30, 2020

The latest beta version of the cli spins a working project for me 👍

Copy link
Contributor

@jeffvg jeffvg left a comment

Choose a reason for hiding this comment

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

all good with latest. Verified pc/mac

@joebochill joebochill merged commit 6164fbb into dev Oct 31, 2020
@joebochill joebochill deleted the bug/fix-react-imports branch October 31, 2020 01:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants