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

Ship merge, mergeInto, copyProperties to npm #2317

Merged
merged 1 commit into from
Oct 14, 2014

Conversation

zpao
Copy link
Member

@zpao zpao commented Oct 10, 2014

This makes sure merge, mergeInto, copyProperties end up in build/modules which then gets packaged up for npm. Is there anything we're missing?

Ideally we should have deprecation notices. But we've never officially supported these. Also, merge now uses Object.assign so anybody using these without polyfills will have a Bad Time™.

Reviewers: @spicyj @sebmarkbage @syranide

Test Plan: grunt build and make sure files are in build/modules/npm-react/lib (before: they weren't)

@zpao zpao added this to the 0.12 milestone Oct 10, 2014
@sophiebits
Copy link
Collaborator

Any reason we can't add a deprecation warning on import?

@zpao
Copy link
Member Author

zpao commented Oct 10, 2014

Yea, we could. I can move the files so it's obvious they diverged (maybe put them in src/vendor_deprecated or something).

@syranide
Copy link
Contributor

👍 This is just a courtesy for those who reached into react/lib/* right?

@sebmarkbage
Copy link
Collaborator

The warnings would be nice.

@zpao
Copy link
Member Author

zpao commented Oct 13, 2014

Added deprecation warnings and READMEs for the people who venture into those vendor dirs.


// deprecation notice
console.warn(
'react/lib/merge has been deprecated and will be removed in the ' +
Copy link
Collaborator

Choose a reason for hiding this comment

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

mergeInto

People may have a bad time if they're depending on these but we can
delay that a little bit. Unless they're using directly and not
polyfilling Object.assign.
zpao added a commit that referenced this pull request Oct 14, 2014
Ship merge, mergeInto, copyProperties to npm
@zpao zpao merged commit 27cbd71 into facebook:master Oct 14, 2014
@zpao zpao deleted the npm-ship-merge branch October 30, 2014 01:07
doctyper pushed a commit to nfl/react-helmet that referenced this pull request Jun 28, 2015
React’s official policy on these helpers is “use at your own risk”:
- facebook/react#1906 (comment)
- facebook/react#2251 (comment)
- facebook/react#2317
power1220 added a commit to power1220/react-helmet that referenced this pull request Apr 4, 2023
React’s official policy on these helpers is “use at your own risk”:
- facebook/react#1906 (comment)
- facebook/react#2251 (comment)
- facebook/react#2317
LucSPI added a commit to LucSPI/React-Helmet that referenced this pull request Feb 12, 2024
React’s official policy on these helpers is “use at your own risk”:
- facebook/react#1906 (comment)
- facebook/react#2251 (comment)
- facebook/react#2317
AMagicHarry added a commit to AMagicHarry/react-helmet that referenced this pull request Mar 11, 2024
React’s official policy on these helpers is “use at your own risk”:
- facebook/react#1906 (comment)
- facebook/react#2251 (comment)
- facebook/react#2317
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