Skip to content

Conversation

@dozoisch
Copy link
Contributor

Motivation

The prop validation was run on every render, even in prod which seems odd to me. This adds a dev check to make sure prod build skips the validation.

Test plan (required)

There are no changes to the ui.

- the input component is validating props in production on each render which
didn't seem very efficient. This ensures it's only run in development.
@dozoisch dozoisch changed the title Add __DEV__ around proptype check Add __DEV__ around textInput prop check Jun 29, 2016
@ghost
Copy link

ghost commented Jun 29, 2016

By analyzing the blame information on this pull request, we identified @JoelMarcey and @Bhullnatik to be potential reviewers.

@ghost ghost added GH Review: review-needed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Jun 29, 2016
@dozoisch dozoisch changed the title Add __DEV__ around textInput prop check [TextInput] Add __DEV__ around textInput prop check Jun 30, 2016
@javache
Copy link
Member

javache commented Jul 1, 2016

@facebook-github-bot shipit

@ghost ghost added GH Review: accepted Import Started This pull request has been imported. This does not imply the PR has been approved. and removed GH Review: review-needed labels Jul 1, 2016
@ghost
Copy link

ghost commented Jul 1, 2016

Thanks for importing. If you are an FB employee go to Phabricator to review.

@ghost ghost closed this in 508cc06 Jul 1, 2016
@dozoisch dozoisch deleted the input_dev_prop_check branch July 1, 2016 16:08
@ide
Copy link
Contributor

ide commented Jul 1, 2016

Can you change this from an error to a warning? Really don't want different failure modes between dev and prod.

@dozoisch
Copy link
Contributor Author

dozoisch commented Jul 1, 2016

@ide if there's a consensus, I don't mind making another PR!

@javache
Copy link
Member

javache commented Jul 4, 2016

@ide this seems similar to how we do other propType validation, so I think it's fine. https://github.com/facebook/react/blob/master/src/isomorphic/classic/types/ReactPropTypes.js#L149

@ide
Copy link
Contributor

ide commented Jul 5, 2016

@javache React creates Errors in order to capture the stack trace but it doesn't throw them. It prints a warning during propType validation: https://github.com/facebook/react/blob/95ac239cf3c527b70daeb433b7aeeaf9e41be9e5/src/isomorphic/classic/types/checkReactTypeSpec.js#L84

@dozoisch
Copy link
Contributor Author

dozoisch commented Jul 5, 2016

@ide so should we change it so that it only prints it instead of throwing?

@ide
Copy link
Contributor

ide commented Jul 5, 2016

Yea I think we want to follow what React does with warning.

dozoisch added a commit to dozoisch/react-native that referenced this pull request Jul 8, 2016
- the goal is to have the same failure mode in prod and dev. See conv in facebook#8499
@dozoisch
Copy link
Contributor Author

dozoisch commented Jul 8, 2016

@ide dozoisch@534d6ce would that be ok with you? If yes, I'll create a pr. Sorry for not responding, never saw your response 3 days ago!

dozoisch added a commit to dozoisch/react-native that referenced this pull request Jul 13, 2016
- the goal is to have the same failure mode in prod and dev. See conv in facebook#8499
ghost pushed a commit that referenced this pull request Jul 13, 2016
Summary:
Follow up of #8499! Open to discussion

Motivation:

See #8499 (comment)

Goal is to have the same failure mode in dev and prod.

/cc ide
Closes #8757

Differential Revision: D3558991

fbshipit-source-id: c7d133f958e67ab23da486b1ffcb8f9963509b79
bubblesunyum pushed a commit to iodine/react-native that referenced this pull request Aug 23, 2016
Summary:
**Motivation**

The prop validation was run on every render, even in prod which seems odd to me. This adds a dev check to make sure prod build skips the validation.

**Test plan (required)**

There are no changes to the ui.
Closes facebook#8499

Differential Revision: D3508805

Pulled By: javache

fbshipit-source-id: 68c6a1224e33997f9df93481426daff790ef5bcd
bubblesunyum pushed a commit to iodine/react-native that referenced this pull request Aug 23, 2016
Summary:
Follow up of facebook#8499! Open to discussion

Motivation:

See facebook#8499 (comment)

Goal is to have the same failure mode in dev and prod.

/cc ide
Closes facebook#8757

Differential Revision: D3558991

fbshipit-source-id: c7d133f958e67ab23da486b1ffcb8f9963509b79
mpretty-cyro pushed a commit to HomePass/react-native that referenced this pull request Aug 25, 2016
Summary:
**Motivation**

The prop validation was run on every render, even in prod which seems odd to me. This adds a dev check to make sure prod build skips the validation.

**Test plan (required)**

There are no changes to the ui.
Closes facebook#8499

Differential Revision: D3508805

Pulled By: javache

fbshipit-source-id: 68c6a1224e33997f9df93481426daff790ef5bcd
mpretty-cyro pushed a commit to HomePass/react-native that referenced this pull request Aug 25, 2016
Summary:
Follow up of facebook#8499! Open to discussion

Motivation:

See facebook#8499 (comment)

Goal is to have the same failure mode in dev and prod.

/cc ide
Closes facebook#8757

Differential Revision: D3558991

fbshipit-source-id: c7d133f958e67ab23da486b1ffcb8f9963509b79
tungdo194 pushed a commit to tungdo194/rn-test that referenced this pull request Apr 28, 2024
Summary:
**Motivation**

The prop validation was run on every render, even in prod which seems odd to me. This adds a dev check to make sure prod build skips the validation.

**Test plan (required)**

There are no changes to the ui.
Closes facebook/react-native#8499

Differential Revision: D3508805

Pulled By: javache

fbshipit-source-id: 68c6a1224e33997f9df93481426daff790ef5bcd
tungdo194 pushed a commit to tungdo194/rn-test that referenced this pull request Apr 28, 2024
Summary:
Follow up of facebook/react-native#8499! Open to discussion

Motivation:

See facebook/react-native#8499 (comment)

Goal is to have the same failure mode in dev and prod.

/cc ide
Closes facebook/react-native#8757

Differential Revision: D3558991

fbshipit-source-id: c7d133f958e67ab23da486b1ffcb8f9963509b79
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Import Started This pull request has been imported. This does not imply the PR has been approved.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants