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

Update all dependencies #11624

Merged
merged 4 commits into from
Dec 2, 2021
Merged

Update all dependencies #11624

merged 4 commits into from
Dec 2, 2021

Conversation

jd1048576
Copy link
Contributor

@jd1048576 jd1048576 commented Nov 5, 2021

I have attempted to update all dependencies to as new as possible, there are a few packages now that are esm only so I have incremented them to the latest available commonjs versions. I have verified my changes by running the integration tests as well as the e2e tests using github actions, the macos node 16 build failed but that seems to be the same as what is on main. I held off on updating eslint to 8 due to the the incompatibility still of the plugins with 8. I had to update the hashes for the css modules, I assume something has changed in webpack since the changes in the module itself don't look like they should affect the hash.

Edit 12 November: I have now updated eslint to 8 as all plugins now seem to be compatible (#11374), the peer dependency also had to be increased as eslint-plugin-flowtype@8 is only compatible with eslint 8. Note it probably worth looking at the config to see if any new rules should be enabled.

image

@jd1048576 jd1048576 requested a review from iansu as a code owner November 5, 2021 15:34
@jd1048576 jd1048576 changed the title Update dependencies Update all dependencies Nov 12, 2021
@GoetzGoerisch
Copy link

@jd1048576 great, could you also include the update [email protected] to fix the JSONPOINTER vulnerability: https://security.snyk.io/vuln/SNYK-JS-JSONPOINTER-1577288

@jd1048576
Copy link
Contributor Author

@GoetzGoerisch I have updated the pr to include [email protected]

@iansu
Copy link
Contributor

iansu commented Nov 17, 2021

Looks like a couple of tests are still failing on macOS. If we can get those fixed then I think we can merge this.

@jd1048576
Copy link
Contributor Author

jd1048576 commented Nov 18, 2021

@iansu I don't think the test failures are due to the changes here, I have updated the timeout to 10 minutes for the macOS runner as it seems to have a high variability in test times resulting in jest timing out.

Here is the output from https://github.com/jd1048576/create-react-app/runs/4247435239?check_suite_focus=true

PASS test/integration/create-react-app/index.test.js (567.808 s)
create-react-app
✓ asks to supply an argument if none supplied (239 ms)
✓ creates a project on supplying a name as the argument (344465 ms)
✓ warns about conflicting files in path (5464 ms)
✓ creates a project in the current directory (53972 ms)
✓ uses yarn as the package manager (99955 ms)
✓ creates a project based on the typescript template (57753 ms)

@GoetzGoerisch
Copy link

GoetzGoerisch commented Nov 18, 2021

@iansu could we please get this merged and build/publish a new 5.0.0-next version?

@jd1048576 thanks for this! 👍🏻

@raix raix added this to the 5.0 milestone Nov 27, 2021
@raix
Copy link
Contributor

raix commented Nov 27, 2021

@jd1048576 Thank you for the pr! We plan to get the eslint v8 in #11375 - the failing tests might be related to eslint. If you are up for it we could try removing the eslint updates from this pr to see if it would fix the tests?
cc @iansu

@jd1048576
Copy link
Contributor Author

@raix I can attempt to remove the eslint specific updates if you want or can wait for #11375 and rebase, I don't however think the changes made hear are specific to test failures as they all passed in or the mock pr I created on my fork after I updated the timeout for the macOS github runner https://github.com/jd1048576/create-react-app/actions/runs/1507045894

@raix
Copy link
Contributor

raix commented Nov 27, 2021

@jd1048576 seems like tests are also failing on main... let's get #11375 in first then rebase - I'm currently trying to fix the integration tests.

Copy link
Contributor

@iansu iansu left a comment

Choose a reason for hiding this comment

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

This looks great. If you can remove the ESLint upgrade then I think we can merge this.

packages/eslint-config-react-app/package.json Outdated Show resolved Hide resolved
@raix
Copy link
Contributor

raix commented Dec 1, 2021

Tests should be fixed on main now 🤞

@jd1048576
Copy link
Contributor Author

@iansu I have reverted eslint back to 8.3.0 after rebasing due to #11375 being merged so hopefully this pr is ok now. @raix I noticed that @svgr/webpack@6 was released since the pr was opened and I attempted to see if I can also include it here but I believe it requires a few changes to the webpack config which I could not get working easily so its probably worth doing that in a follow up pr

@MichaelDeBoey
Copy link
Contributor

@iansu Seems like all tests are green, so this one can be merged too I guess

@iansu iansu merged commit 3afbbc0 into facebook:main Dec 2, 2021
@iansu
Copy link
Contributor

iansu commented Dec 2, 2021

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants