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

Migrate ReactNative View.propTypes to ViewPropTypes #99

Merged
merged 4 commits into from
Mar 23, 2017

Conversation

bvaughn
Copy link
Contributor

@bvaughn bvaughn commented Mar 23, 2017

ReactNative will soon be deprecating View.propTypes in favor of ViewPropTypes. This codemod helps with that deprecation by doing the following:

  • Updates all references to View.propTypes and replaces them with ViewPropTypes.
  • Adds an import or require statement (depending on the convention followed in the parent file) for the new ViewPropTypes.
  • Removes the import/require statement for View if it is no longer in use.
  • Uses Haste modules (eg import ViewPropTypes from 'ViewPropTypes';) if it detects their use in the file else uses Node modules (eg import { ViewPropTypes } from 'react-native';).

Testing

In addition to the newly added Jest tests this codemod was also run internally against Facebook's ReactNative code.

Caveats

This codemod may introduce an unnecessary newline before certain types of imports. This is not a problem with the codemod but with recast. See facebook/jscodeshift/issues/185 and benjamn/recast/issues/371

This codemod doesn't handle the case demonstrated below:

const ReactNative = require('react-native');
const { View } = ReactNative;

This pattern isn't common within Facebook and I didn't think it would be worth the extra complexity it would add to the codemod. If others feel strongly about this I could add support for it.

@bvaughn bvaughn requested a review from acdlite March 23, 2017 18:32
necolas added a commit to necolas/react-native-web that referenced this pull request Mar 23, 2017
Copy link
Member

@acdlite acdlite left a comment

Choose a reason for hiding this comment

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

LGTM

@acdlite
Copy link
Member

acdlite commented Mar 23, 2017

🎉

@bvaughn bvaughn merged commit 9b1d355 into reactjs:master Mar 23, 2017
@bvaughn bvaughn deleted the ReactNative-View-propTypes branch March 23, 2017 21:17
@keyz
Copy link
Member

keyz commented Mar 23, 2017

We probably don't have this pattern internally so it's fine, but if const View = require('View') is the very first line of a program after some comments (like @flow or @providesModule) then replacing it will remove the comments (example). Just FYI -- I have broken Haste a few times because of this :p

@bvaughn
Copy link
Contributor Author

bvaughn commented Mar 23, 2017

That's interesting. Good thing to be aware of, @keyanzhang.

When scanning the diff I ran internally I didn't see any such substitutions FWIW.

@sebmarkbage
Copy link

In open source usage people aren't allowed to reference ViewPropTypes directly because CommonJS instead of Haste. Except in the internals of react-native. How is open source going to require this?

require('react-native').ViewPropTypes

@bvaughn
Copy link
Contributor Author

bvaughn commented Mar 24, 2017

@sebmarkbage Yeah, I added a getter for ViewPropTypes to react-native-implementation so the require the codemod writes for non-Haste usage should work fine.

james424smith added a commit to james424smith/react-native-web that referenced this pull request Oct 28, 2020
Serhey-Oleynik added a commit to Serhey-Oleynik/react-native-app that referenced this pull request Apr 10, 2022
superstar1205 added a commit to superstar1205/ReactNative-Web that referenced this pull request Dec 26, 2022
power1220 added a commit to power1220/react-native-web that referenced this pull request Apr 4, 2023
crown0128 added a commit to crown0128/react-native-web that referenced this pull request Aug 12, 2023
Hiccup19940325 pushed a commit to Hiccup19940325/reactnative_package that referenced this pull request Oct 30, 2023
smartDev420 pushed a commit to smartDev420/ReactNative-Web-App that referenced this pull request Aug 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants