Skip to content

Conversation

@NMinhNguyen
Copy link
Collaborator

See facebook/react#18145

Test Plan

I've verified this locally and the build outputs aren't changed due to this. They're only slightly different due to the dependency updates.

@NMinhNguyen NMinhNguyen force-pushed the esm branch 2 times, most recently from adde915 to d2d678e Compare February 27, 2020 13:25
@ljharb
Copy link
Member

ljharb commented Feb 27, 2020

It’d be great to discuss major changes like this before making them; enzyme will need to use a shallow renderer that’s developed using the same standards enzyme itself is.

@NMinhNguyen
Copy link
Collaborator Author

@ljharb could you clarify why this would affect enzyme's usage of this package at all? I have verified the build outputs and there shouldn't be any difference, i.e. there's no need to do require('react-shallow-renderer').default if that's what you're worried about?

@gaearon
Copy link
Collaborator

gaearon commented Feb 27, 2020

Sorry we didn't communicate this clearly; this is mirroring the change we did in the React repo: facebook/react#18145. It shouldn't affect the build in any meaningful way for the CJS mode.

@ljharb
Copy link
Member

ljharb commented Feb 27, 2020

@gaearon is the intention for Facebook to continue maintaining a parallel implementation, or will this become the canonical one?

@gaearon
Copy link
Collaborator

gaearon commented Feb 27, 2020

The plan is for this to be the canonical one.

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