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

Framework: Update React addons dependency if addons are unused #1917

Merged
merged 4 commits into from
Dec 23, 2015

Conversation

aduth
Copy link
Contributor

@aduth aduth commented Dec 22, 2015

Related: #1498, #1878, #1901

This pull request seeks to change the declared dependency of React from react/addons to react if the file does not use any methods from React.addons. React addons were moved to separate packages in React 0.14 and currently cause warnings to appear in development environments.

Implementation notes:

These replacements were made programmatically using the following script: https://gist.github.com/aduth/8fee70ba5719d48d6b7f

Testing instructions:

Assuming the replacements are valid, there should be no effective change in the application.

@aduth aduth added Framework [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Dec 22, 2015
@aduth
Copy link
Contributor Author

aduth commented Dec 22, 2015

I modified the script slightly to try to find cases where React addons were used but not declared, which should have been fixed as part of #1625, but I found one case remaining, which I included here in c6d1681.

@aduth
Copy link
Contributor Author

aduth commented Dec 22, 2015

Looks like the script had been a bit overzealous in the case where addons was used directly from the require statement. This is pretty poor form (which I can say as the author of the case 😄 ), and is fixed/improved in 23c51b0. Also did a manual scan over all remaining instances of .addons to confirm there are no other cases like this.

@ockham
Copy link
Contributor

ockham commented Dec 22, 2015

Tested, read through all remaining .addons (whew), and pushed a really minor fix (41ff5b1).

:shipit:

@ockham ockham added [Status] Ready to Merge and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Dec 22, 2015
@ockham ockham force-pushed the remove/react-addons-unused branch from 41ff5b1 to a4bcc71 Compare December 23, 2015 10:48
@ockham ockham force-pushed the remove/react-addons-unused branch from a4bcc71 to 96396db Compare December 23, 2015 10:50
@ockham
Copy link
Contributor

ockham commented Dec 23, 2015

Rebased.

@ehg
Copy link
Member

ehg commented Dec 23, 2015

Rebased.

👍

ockham added a commit that referenced this pull request Dec 23, 2015
Framework: Update React addons dependency if addons are unused
@ockham ockham merged commit 2bc5d08 into master Dec 23, 2015
@ockham ockham deleted the remove/react-addons-unused branch December 23, 2015 11:14
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.

4 participants