Skip to content

Convert React.createClass to React.Component#877

Merged
ljharb merged 1 commit intoenzymejs:masterfrom
koba04:convert-es2015-class
Apr 10, 2017
Merged

Convert React.createClass to React.Component#877
ljharb merged 1 commit intoenzymejs:masterfrom
koba04:convert-es2015-class

Conversation

@koba04
Copy link
Copy Markdown
Contributor

@koba04 koba04 commented Apr 10, 2017

In react@15.5, React.createClass is deprecated, So we have to use create-react-class instead. #876

In this PR, I've converted React.createClass to React.Component except some tests for React.createClass.
Should Enzyme support create-react-class? If it isn't necessary, I can remove all React.createClass from the tests.

After this PR is merged, Enzyme can put create-react-class in devDependencies instead of dependencies.

@koba04 koba04 mentioned this pull request Apr 10, 2017
Merged
ljharb
ljharb requested changes Apr 10, 2017
View reviewed changes
Copy link
Copy Markdown
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

Please revert all the non-docs changes, and then this can land in master. #876 doesn't change the need to test, forever, createClass classes, so tests should continue to test them.

src/ReactWrapperComponent.jsx

getInitialState() {
return {
class WrapperComponent extends React.Component {
Copy link
Copy Markdown
Member

@ljharb ljharb Apr 10, 2017

Choose a reason for hiding this comment

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

we definitely shouldn't change the specs here; we should continue to use createClass.

Copy link
Copy Markdown
Contributor Author

@koba04 koba04 Apr 10, 2017

Choose a reason for hiding this comment

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

@ljharb What's the reason to continue to use createClass? It seems to be able to replace with ES2015 classes. Is there any problems?

Copy link
Copy Markdown
Contributor Author

@koba04 koba04 Apr 10, 2017

Choose a reason for hiding this comment

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

I think it would be nice if Enzyme users don't have to install create-react-class to use it.

Copy link
Copy Markdown
Member

@ljharb ljharb Apr 10, 2017

Choose a reason for hiding this comment

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

oops, you're right, this one isn't a spec file - this can stay as-is :-)

nfcampos
nfcampos reviewed Apr 10, 2017
View reviewed changes
Copy link
Copy Markdown
Collaborator

@nfcampos nfcampos left a comment

Choose a reason for hiding this comment

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

i'd you undo the changes to the test files this can be merged

docs/api/ShallowWrapper/matchesElement.md
const MyComponent = React.createClass({
class MyComponent extends React.Component {
handleClick() {
...
Copy link
Copy Markdown
Collaborator

@nfcampos nfcampos Apr 10, 2017

Choose a reason for hiding this comment

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

this component doesn't really work now that it's a class because methods aren't auto bound, so if this is used in handleClick it won't work.

do we want to have an "invalid" component as an example in the docs?

Copy link
Copy Markdown
Member

@ljharb ljharb Apr 10, 2017

Choose a reason for hiding this comment

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

good catch - let's add the best practice of a manual constructor binding to it.

Copy link
Copy Markdown
Contributor Author

@koba04 koba04 Apr 10, 2017

Choose a reason for hiding this comment

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

That makes sense. I've fixed it.

@koba04 koba04 force-pushed the convert-es2015-class branch from 0056359 to a9c7b66 Compare April 10, 2017 06:23
@koba04 koba04 force-pushed the convert-es2015-class branch from a9c7b66 to 0dfdd84 Compare April 10, 2017 06:40
@koba04
Copy link
Copy Markdown
Contributor Author

koba04 commented Apr 10, 2017

Ok, I've reverted changes for tests.

But I think it's good to use React.createClass only in tests that React.createClass is required because React is going to separate React.createClass from React core.

@nfcampos
Copy link
Copy Markdown
Collaborator

nfcampos commented Apr 10, 2017

LGTM

nfcampos added a commit to kentcdodds/enzyme that referenced this pull request Apr 10, 2017
@nfcampos
Remove dependency on create-react-class
3a24258 
- remove createClass export from src/react-compat
- add test/react-compat with createClass export
- change spec files to import createClass from test/react-compat
- undo changes to src/ReactWrapperComponent.jsx (use of createClass there removed in enzymejs#877
- update README with correct dependencies for react@15.5
@ljharb ljharb merged commit c8f2185 into enzymejs:master Apr 10, 2017
nfcampos added a commit to kentcdodds/enzyme that referenced this pull request Apr 10, 2017
@nfcampos
Remove dependency on create-react-class
107e2e6 
- remove createClass export from src/react-compat
- add test/react-compat with createClass export
- change spec files to import createClass from test/react-compat
- undo changes to src/ReactWrapperComponent.jsx (use of createClass there removed in enzymejs#877
- update README with correct dependencies for react@15.5
@koba04
Copy link
Copy Markdown
Contributor Author

koba04 commented Apr 10, 2017

Thanks!

@koba04 koba04 deleted the convert-es2015-class branch April 10, 2017 09:48
nfcampos added a commit to kentcdodds/enzyme that referenced this pull request Apr 10, 2017
@nfcampos
Remove dependency on create-react-class
403225d 
- remove createClass export from src/react-compat
- add test/react-compat with createClass export
- change spec files to import createClass from test/react-compat
- undo changes to src/ReactWrapperComponent.jsx (use of createClass there removed in enzymejs#877
- update README with correct dependencies for react@15.5
ljharb pushed a commit to kentcdodds/enzyme that referenced this pull request Apr 12, 2017
@nfcampos @ljharb
Remove dependency on create-react-class
1fafc7f 
- remove createClass export from src/react-compat
- add test/react-compat with createClass export
- change spec files to import createClass from test/react-compat
- undo changes to src/ReactWrapperComponent.jsx (use of createClass there removed in enzymejs#877
- update README with correct dependencies for react@15.5
ljharb pushed a commit to kentcdodds/enzyme that referenced this pull request Apr 12, 2017
@nfcampos @ljharb
Remove dependency on create-react-class
edeb99c 
- remove createClass export from src/react-compat
- add test/react-compat with createClass export
- change spec files to import createClass from test/react-compat
- undo changes to src/ReactWrapperComponent.jsx (use of createClass there removed in enzymejs#877
- update README with correct dependencies for react@15.5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ljharb ljharb ljharb approved these changes

@nfcampos nfcampos nfcampos approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@koba04 @nfcampos @ljharb