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.

Sign up for GitHub

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

Uninstall unused Webpack 4 packages #27058

Merged
merged 3 commits into from
Jul 25, 2023
Merged

Uninstall unused Webpack 4 packages #27058

merged 3 commits into from
Jul 25, 2023

Conversation

anatoliik-lyft
Copy link
Contributor

@anatoliik-lyft anatoliik-lyft commented Jul 6, 2023 •
edited
Loading

Summary

  • Remove unused webpack 4 dependencies

How did you test this change?

  • Ran yarn test --prod
  • Ran yarn test

Related PRs:

@anatoliik-lyft anatoliik-lyft changed the title Use webpack 5 Uninstall unused Webpack 4 packages Jul 6, 2023
@react-sizebot
Copy link

react-sizebot commented Jul 6, 2023 •
edited
Loading

Comparing: 035a41c...e3d7189

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.min.js = 164.35 kB 164.35 kB = 51.76 kB 51.76 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 171.76 kB 171.76 kB = 53.98 kB 53.98 kB
facebook-www/ReactDOM-prod.classic.js = 567.22 kB 567.22 kB = 100.04 kB 100.04 kB
facebook-www/ReactDOM-prod.modern.js = 551.02 kB 551.02 kB = 97.21 kB 97.21 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against e3d7189

@anatoliik-lyft
Copy link
Contributor Author

👋 @hoxyq tagging you in case you want to take a look, this PR is to replace webpack 4 in the root with webpack 5 and use the root one from workspaces. Would be awesome if you can take a look to make sure I didn't brake anything 🙌

@hoxyq
Copy link
Contributor

hoxyq commented Jul 7, 2023

👋 @hoxyq tagging you in case you want to take a look, this PR is to replace webpack 4 in the root with webpack 5 and use the root one from workspaces. Would be awesome if you can take a look to make sure I didn't brake anything 🙌

Hey, thanks for working on this!

I am not sure this is what we want though. These are devDependencies of specific packages inside the monorepo, react-devtools-* in this case. These are not dependencies of the react monorepo itself.

It is also questionable why do we need webpack listed here in root package.json, need to check if we use webpack somewhere from root node_modules. If not, then we can safely remove it.

@hoxyq
Copy link
Contributor

hoxyq commented Jul 7, 2023

👋 @hoxyq tagging you in case you want to take a look, this PR is to replace webpack 4 in the root with webpack 5 and use the root one from workspaces. Would be awesome if you can take a look to make sure I didn't brake anything 🙌

Hey, thanks for working on this!

I am not sure this is what we want though. These are devDependencies of specific packages inside the monorepo, react-devtools-* in this case. These are not dependencies of the react monorepo itself.

It is also questionable why do we need webpack listed here in root package.json, need to check if we use webpack somewhere from root node_modules. If not, then we can safely remove it.

Also, having this dependency on a root level introduces a new constraint: imagine updating webpack to a new major version, say 6, you would need to check that this change doesn't break dozens of packages that might depend on webpack.

@anatoliik-lyft
Copy link
Contributor Author

thanks for the quick response! that makes sense! let me update the PR to just remove webpack 4 from the root and see if it works

@hoxyq
Copy link
Contributor

hoxyq commented Jul 9, 2023

cc @sebmarkbage

Can we remove this root level webpack dependency or am I missing something? This was added in #17379

hoxyq
hoxyq approved these changes Jul 25, 2023
View reviewed changes
Copy link
Contributor

@hoxyq hoxyq left a comment

Choose a reason for hiding this comment

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

Looks safe to me.

@hoxyq hoxyq merged commit fdb368d into facebook:main Jul 25, 2023
2 checks passed
@gnoff gnoff mentioned this pull request Aug 8, 2023
Merged
kodiakhq bot pushed a commit to vercel/next.js that referenced this pull request Aug 8, 2023
@gnoff
update vendored React packages to the the latest canary (#53742)
f0dab3a 
Updated React from 9377e1010 to cb3404a0c.

### React upstream changes

- facebook/react#27190
- facebook/react#27189
- facebook/react#27201
- facebook/react#27147
- facebook/react#26949
- facebook/react#27058
- facebook/react#27142
- facebook/react#27133
- facebook/react#27130
- facebook/react#27105
- facebook/react#27117
- facebook/react#27057
EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024
@anatoliik-lyft @AndyPengc12
Uninstall unused Webpack 4 packages (facebook#27058)
132d5a3 
<!--
  Thanks for submitting a pull request!
We appreciate you spending the time to work on these changes. Please
provide enough information so that others can review your pull request.
The three fields below are mandatory.

Before submitting a pull request, please make sure the following is
done:

1. Fork [the repository](https:/facebook/react) and create
your branch from `main`.
  2. Run `yarn` in the repository root.
3. If you've fixed a bug or added code that should be tested, add tests!
4. Ensure the test suite passes (`yarn test`). Tip: `yarn test --watch
TestName` is helpful in development.
5. Run `yarn test --prod` to test in the production environment. It
supports the same options as `yarn test`.
6. If you need a debugger, run `yarn test --debug --watch TestName`,
open `chrome://inspect`, and press "Inspect".
7. Format your code with
[prettier](https:/prettier/prettier) (`yarn prettier`).
8. Make sure your code lints (`yarn lint`). Tip: `yarn linc` to only
check changed files.
  9. Run the [Flow](https://flowtype.org/) type checks (`yarn flow`).
  10. If you haven't already, complete the CLA.

Learn more about contributing:
https://reactjs.org/docs/how-to-contribute.html
-->

## Summary

 - Remove unused webpack 4 dependencies

## How did you test this change?

 - Ran `yarn test --prod`
 - Ran `yarn test`

## Related PRs:
 - facebook#26887
bigfootjon pushed a commit that referenced this pull request Apr 18, 2024
@hoxyq
Uninstall unused Webpack 4 packages (#27058)
900caee 
<!--
  Thanks for submitting a pull request!
We appreciate you spending the time to work on these changes. Please
provide enough information so that others can review your pull request.
The three fields below are mandatory.

Before submitting a pull request, please make sure the following is
done:

1. Fork [the repository](https:/facebook/react) and create
your branch from `main`.
  2. Run `yarn` in the repository root.
3. If you've fixed a bug or added code that should be tested, add tests!
4. Ensure the test suite passes (`yarn test`). Tip: `yarn test --watch
TestName` is helpful in development.
5. Run `yarn test --prod` to test in the production environment. It
supports the same options as `yarn test`.
6. If you need a debugger, run `yarn test --debug --watch TestName`,
open `chrome://inspect`, and press "Inspect".
7. Format your code with
[prettier](https:/prettier/prettier) (`yarn prettier`).
8. Make sure your code lints (`yarn lint`). Tip: `yarn linc` to only
check changed files.
  9. Run the [Flow](https://flowtype.org/) type checks (`yarn flow`).
  10. If you haven't already, complete the CLA.

Learn more about contributing:
https://reactjs.org/docs/how-to-contribute.html
-->

## Summary

 - Remove unused webpack 4 dependencies

## How did you test this change?

 - Ran `yarn test --prod`
 - Ran `yarn test`

## Related PRs:
 - #26887

DiffTrain build for commit fdb368d.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hoxyq hoxyq hoxyq approved these changes

Assignees
No one assigned
Labels
CLA Signed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@anatoliik-lyft @react-sizebot @hoxyq @facebook-github-bot