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

Remove unused dependency 'abort-controller' #26074

Merged
merged 2 commits into from
Jan 31, 2023
Merged

Conversation

ymqy
Copy link
Contributor

@ymqy ymqy commented Jan 29, 2023

Summary

This PR removes the unused dependency 'abort-controller' from the project. it helps to keep the project clean and maintainable.

How did you test this change?

ci green

@react-sizebot
Copy link

react-sizebot commented Jan 29, 2023

Comparing: d7bb524...a299e26

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 = 154.83 kB 154.83 kB = 49.11 kB 49.11 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 156.83 kB 156.83 kB = 49.77 kB 49.77 kB
facebook-www/ReactDOM-prod.classic.js = 533.79 kB 533.79 kB = 95.06 kB 95.06 kB
facebook-www/ReactDOM-prod.modern.js = 518.81 kB 518.81 kB = 92.83 kB 92.83 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against a299e26

@ymqy ymqy changed the title [cleanup] Remove unused dependencies and update 'flowconfig' [checkout code error💥][cleanup] Remove unused dependencies and update 'flowconfig' Jan 30, 2023
@ymqy ymqy changed the title [checkout code error💥][cleanup] Remove unused dependencies and update 'flowconfig' [cleanup] Remove unused dependencies and update 'flowconfig' Jan 30, 2023
@ymqy ymqy changed the title [cleanup] Remove unused dependencies and update 'flowconfig' [cleanup] Remove unused dependencies and update 'flowconfig' ignore Jan 30, 2023
@ymqy ymqy changed the title [cleanup] Remove unused dependencies and update 'flowconfig' ignore Remove unused dependency 'abort-controller' Jan 30, 2023
@@ -36,7 +36,6 @@
"@babel/preset-flow": "^7.10.4",
"@babel/preset-react": "^7.10.4",
"@babel/traverse": "^7.11.0",
"abort-controller": "^3.0.0",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Added in #21027 but it doesn't look like it was even used in that PR. Maybe something used during development?

Copy link
Contributor Author

@ymqy ymqy Jan 31, 2023

Choose a reason for hiding this comment

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

@eps1lon 'abort-controller' was used in packages/react-dom/src/tests/ReactDOMFizzServerBrowser-test.js, but was deleted in #24285, because it added a lightweight fallback shim for environments that don't have an AbortController (e.g. tests running in Node < 15).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hm. Interestingly that fix is only on the client. It didn’t necessarily fix the server. I guess we just don’t run tests in earlier node environments and we don’t really support them anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It should be safe to remove this dependency

Copy link
Member

Choose a reason for hiding this comment

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

Given that nothing requires this dependency at the moment, it seems like we can remove it at least until we do require it again. Seems like that might be required for some old node versions, but it seems like they'd need to be tested more extensively anyway.

If someone want's to use abort-controller again, it'd easy to add back the dependency. I'll merge this.

@eps1lon eps1lon requested a review from sebmarkbage January 31, 2023 04:49
@kassens kassens merged commit 8b9ac81 into facebook:main Jan 31, 2023
@ymqy ymqy deleted the cleanup branch January 31, 2023 15:48
github-actions bot pushed a commit that referenced this pull request Jan 31, 2023
## Summary
This PR removes the unused dependency 'abort-controller' from the
project. it helps to keep the project clean and maintainable.

## How did you test this change?
ci green

DiffTrain build for [8b9ac81](8b9ac81)
[View git log for this commit](https://github.com/facebook/react/commits/8b9ac8175fb3052ba5c82f99003bb8ff667e1e68)
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.

6 participants