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

Rename internal fields #17798

Closed

Conversation

sebmarkbage
Copy link
Collaborator

@sebmarkbage sebmarkbage commented Jan 7, 2020

One day we'll use private fields for this but until then I'm just going to rename these every version until people get the hint.

This is actually better than if we change fields while keeping the same name. Because at least this will fail early (and often bailout) where as once found, people tend to assume that the data structure is there and as such throws random errors to users.

One day we'll use private fields for this but until then I'm just going to
rename these every version until people get the hint.
@codesandbox-ci
Copy link

codesandbox-ci bot commented Jan 7, 2020

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit e381e58:

Sandbox Source
vibrant-sunset-sdlo6 Configuration

@sizebot
Copy link

sizebot commented Jan 7, 2020

No significant bundle size changes to report.

Size changes (experimental)

Generated by 🚫 dangerJS against e381e58

@sizebot
Copy link

sizebot commented Jan 7, 2020

No significant bundle size changes to report.

Size changes (stable)

Generated by 🚫 dangerJS against e381e58

@gaearon
Copy link
Collaborator

gaearon commented Jan 7, 2020

I'm just going to rename these every version

Like.. manually?..

Copy link
Collaborator

@gaearon gaearon left a comment

Choose a reason for hiding this comment

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

OK but maybe fix the RN inspector overlay somehow?

@facebook-github-bot facebook-github-bot added the React Core Team Opened by a member of the React Core Team label Jan 8, 2020
Copy link
Collaborator

@gaearon gaearon left a comment

Choose a reason for hiding this comment

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

Let's have a strategy for the cases we found first.

Copy link
Collaborator

@gaearon gaearon left a comment

Choose a reason for hiding this comment

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

Let's get this into the next sync? And fix up the known cases (E2E). We'll need to give heads-up to Enzyme.

@gaearon gaearon mentioned this pull request Mar 24, 2020
@gaearon
Copy link
Collaborator

gaearon commented Apr 1, 2020

We did this.

@gaearon gaearon closed this Apr 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants