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

Planned breakage: React 16.14 compatibility #2358

Open
gaearon opened this issue Mar 25, 2020 · 12 comments
Open

Planned breakage: React 16.14 compatibility #2358

gaearon opened this issue Mar 25, 2020 · 12 comments
Assignees
Projects

Comments

@gaearon
Copy link
Contributor

gaearon commented Mar 25, 2020

We are renaming some internal fields in facebook/react#18377. That commit will likely make it into React 16.14. We don't have a clear timeline on what it'll be out but I would expect within a couple of weeks.

Since the Enzyme 16 adapter depends on this field name, it would either need to be updated or a new adapter for 16.14+ would need to be created.

Please note that in longer term, this isn't really going to be sustainable. We are planning bigger refactors which will change the internal data structure. I wonder if it would be better if Enzyme just shipped with its own copy of React that may lag behind in features. In our experience, Enzyme lagging behind due to its invasive reliance on internals has been the biggest factor in moving away from it.

@ljharb
Copy link
Member

ljharb commented Mar 25, 2020

Thanks for the heads up, I'll try to get updates in that use the new field names when available. If this gets published in a prerelease sooner than an actual release, I can start running tests against that.

The reason things have been hard to sustain is because React doesn't expose sufficient public APIs to properly test it ("just dump it into the dom and look at that" doesn't qualify); shipping its own copy of React isn't going to fix that, and it would worsen how behind we are, since most of the time everything just works. I look forward to finding a better long term solution than relying on internals (#1648), which will hopefully involve more exposed/supported testing hooks (including for hooks themselves, which I don't think have any).

@ljharb ljharb added this to Needs Triage in React 16 via automation Mar 25, 2020
@ljharb ljharb self-assigned this Mar 25, 2020
@gaearon
Copy link
Contributor Author

gaearon commented Mar 25, 2020

Regarding testing Hooks, technically you can always override the Hooks Dispatcher to point to arbitrary implementation. That’s how Hooks work today in different implementations. The Dispatcher exists on a scary hidden property of the React object but that part is actually pretty stable because it exists solely for dependency injection and doesn’t contain any logic.

@gaearon
Copy link
Contributor Author

gaearon commented Mar 25, 2020

Regarding official API. @bvaughn is working on a proposal that adds limited introspection abilities to a special “testing build” of ReactDOM. However they are intentionally much more limited than what Enzyme currently provides. He will share an RFC soon. I imagine it won’t be sufficient for Enzyme. However, there is also an argument that we don’t think exposing other things or relying on them in tests is a necessarily good thing in the first place. Here it might be difficult to find a path forward if we don’t agree on the big picture approach.

@bvaughn
Copy link
Contributor

bvaughn commented Mar 26, 2020

Regarding testing Hooks, technically you can always override the Hooks Dispatcher to point to arbitrary implementation.

This is a dangerous path though, since it requires some internal knowledge about e.g. composite hooks (for example: facebook/react#18130).

He will share an RFC soon.

I think I'm probably not ready yet to share an RFC. (I think I'm going to want to play around with Jest e2e integration with Scott some first and see how that goes.)

If @ljharb would be interested in hopping on video chat sometime to talk about the new APIs at a high level though, I'd be happy to do that. The currently planned set of APIs is actually pretty powerful I think, although there's some question about how much of that "power" we want to expose to users for direct usage (vs maybe something that e.g. Jest e2e internals use for validation). That's part of what I'd like to work out first before an RFC.

@ljharb
Copy link
Member

ljharb commented Mar 26, 2020

I'd be more than happy to do that! It's a totally viable option to release a v4 of enzyme with a more limited featureset, if it means that keeping up to date with React changes will be seamless.

@bvaughn
Copy link
Contributor

bvaughn commented Mar 26, 2020

What's your schedule like Thursday and Friday? Any particular time(s) good for a video chat?

@ljharb

This comment has been minimized.

@bvaughn

This comment has been minimized.

@ljharb

This comment has been minimized.

@ljharb
Copy link
Member

ljharb commented Mar 26, 2020

We had this chat; there's definitely a potential for enzyme to use some of these new APIs, but it unfortunately doesn't solve the majority of what enzyme (and any non-basic testing framework) needs. Hopefully there will be more such APIs in the future and we can close that gap!

@eps1lon
Copy link
Contributor

eps1lon commented Mar 31, 2020

If this gets published in a prerelease sooner than an actual release, I can start running tests against that.

Seems like react@next uses the new names now. Is there any chance enzyme can publish a version that tracks react@next more closely (maybe an adapter?). Looking at the comments in the PR that renames the fields, this will happen again. Would be great if we have a strategy to mitigate these issues in the future. Currently react@next is untestable for us.

@ljharb
Copy link
Member

ljharb commented Mar 31, 2020

Thanks, I'll try to get tests running on next this week (TC39 is today through Thursday, so probably after that).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
React 16
  
Needs Triage
Development

No branches or pull requests

4 participants