Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

React 16 beta #10294

Open
bvaughn opened this Issue Jul 26, 2017 · 192 comments

Comments

Projects
None yet
Contributor

bvaughn commented Jul 26, 2017 edited

The first React 16 beta is now available for public testing. 🎉

Installation Instructions

The beta has been published to NPM with the tag "next". Regular NPM installs will continue to use the 15.6 release. To install the beta use:

yarn add react@next react-dom@next

Or:

npm install --save react@next react-dom@next

What Does React 16 Mean for You?

React 16 is the first release that ships with a rewrite of the React core (previously codenamed “Fiber”). This rewrite had a few goals:

  • Remove old internal abstractions that didn’t age well and hindered internal changes.
  • Let us ship some of the most requested features like returning arrays from render, recovering from component errors, and readable component stack traces for every error.
  • Enable us to start experimenting with asynchronous rendering of components for better perceived performance.

This initial React 16.0 release is mostly focused on compatibility with existing apps. It does not enable asynchronous rendering yet. We will introduce an opt-in to the async mode later during React 16.x. We don’t expect React 16.0 to make your apps significantly faster or slower, but we’d love to know if you see improvements or regressions.

JavaScript Environment Requirements

React 16 depends on the collection types Map and Set. If you support older browsers and devices which may not yet provide these natively (eg <IE11), consider including a global polyfill in your bundled application, such as core-js or babel-polyfill.

A polyfilled environment for React 16 using core-js to support older browsers might look like:

import 'core-js/es6/map';
import 'core-js/es6/set';

import React from 'react';
import ReactDOM from 'react-dom';

ReactDOM.render(
  <h1>Hello, world!</h1>,
  document.getElementById('root')
);

React also depends on requestAnimationFrame (even in test environments). A simple shim for testing environments would be:

global.requestAnimationFrame = function(callback) {
  setTimeout(callback, 0);
};

Points of Interest

  • This is a complete rewrite of React but we expect it to work with your existing code. If you fixed all deprecation warnings introduced in 15.x, the 16 beta should work for you.
  • Third party libraries that relied on deprecated or unsupported APIs may need updates to work correctly with this new release. Now is a good time to file issues against libraries that have problems. (And tell us if we broke something!)
  • We are particularly interested in hearing about performance differences you notice between 15.x and 16.x. We don't expect any massive changes but would love to know about improvements or regressions. Please report them here!
  • The server renderer has been completely rewritten, and now offers a streaming mode (ReactDOMServer.renderToStream() and ReactDOMServer.renderToStaticStream()). Server rendering does not use markup validation anymore, and instead tries its best to attach to existing DOM, warning about inconsistencies. And there is no data-reactid anymore! This server renderer code is still very new, so it is likely to have issues. Please report them.
  • React Native follows a different release cycle and will update to the beta in a future release. (It's already using an alpha release but not yet using Fiber.)

Breaking Changes

Error Handling

  • Previously, runtime errors used to put React into a broken state and produce cryptic errors. React 16 fixes this by introducing a special kind of components called “error boundaries”. Error boundaries can catch runtime errors in a component tree, log them, and display a fallback UI instead.
  • If there is an uncaught error in a component, and there is no error boundary up in the tree, the whole component tree will be unmounted. This helps avoid very nasty bugs where the UI has been corrupted due to an error, but it means that you need to add a few error boundaries to your app to handle the errors gracefully.
  • React 15 had a very limited undocumented support for error boundaries with a different method name. It used to be called unstable_handleError, but has been renamed to componentDidCatch.

You can learn more about the new error handling behavior here.

Scheduling and Lifecycle

  • ReactDOM.render() and ReactDOM.unstable_renderIntoContainer() now return null if called from inside a lifecycle method.
  • setState:
    • Calling setState with null no longer triggers an update. This allows you to decide in an updater function if you want to re-render.
    • Calling setState directly in render always causes an update. This was not previously the case. Regardless, you should not be calling setState from render.
    • setState callback (second argument) now fires immediately after componentDidMount / componentDidUpdate instead of after all components have rendered.
  • When replacing <A /> with <B />, B.componentWillMount now always happens before A.componentWillUnmount. Previously, A.componentWillUnmount could fire first in some cases.
  • Previously, changing the ref to a component would always detach the ref before that component's render is called. Now, we change the ref later, when applying the changes to the DOM.
  • It is not safe to re-render into a container that was modified by something other than React. This worked previously in some cases but was never supported. We now emit a warning in this case. Instead you should clean up your component trees using ReactDOM.unmountComponentAtNode. See this example.
  • componentDidUpdate lifecycle no longer receives prevContext param. (See #8631)
  • Shallow renderer no longer calls componentDidUpdate() because DOM refs are not available. This also makes it consistent with componentDidMount() (which does not get called in previous versions either).
  • Shallow renderer does not implement unstable_batchedUpdates() anymore.

Packaging

  • There is no react/lib/* and react-dom/lib/* anymore. Even in CommonJS environments, React and ReactDOM are precompiled to single files (“flat bundles”). If you previously relied on undocumented React internals, and they don’t work anymore, let us know about your specific case in this issue, and we’ll try to figure out a migration strategy for you.
  • There is no react-with-addons.js build anymore. All compatible addons are published separately on npm, and have single-file browser versions if you need them.
  • The deprecations introduced in 15.x have been removed from the core package. React.createClass is now available as create-react-class, React.PropTypes as prop-types, React.DOM as react-dom-factories, react-addons-test-utils as react-dom/test-utils, and shallow renderer as react-test-renderer/shallow. See 15.5.0 and 15.6.0 blog posts for instructions on migrating code and automated codemods.
  • The names and paths to the single-file browser builds have changed to emphasize the difference between development and production builds. For example:
    • react/dist/react.jsreact/umd/react.development.js
    • react/dist/react.min.jsreact/umd/react.production.min.js
    • react-dom/dist/react-dom.jsreact-dom/umd/react-dom.development.js
    • react-dom/dist/react-dom.min.js → react-dom/umd/react-dom.production.min.js

Known Issues

  • The server renderer crashes in production with inline styles. (#10299, fixed by #10300)
    • Fixed in 16.0.0-beta.2
  • The server renderer doesn't yet support returning arrays and strings from components.
    • Fixed in 16.0.0-beta.3
  • The server renderer still renders data-reactid somewhat unnecessarily. (#10306)
    • Fixed in 16.0.0-beta.3
  • Shallow renderer doesn’t pass context to componentWillReceiveProps (to be fixed by #10342)
    • Fixed in 16.0.0-beta.3
  • There is an issue with 'use strict' in older browsers (#10294 (comment))
    • Fixed in 16.0.0-beta.3
  • In some cases Error: null is reported instead of the real error. (#10321)
    • Fixed in 16.0.0-beta.3
  • There is a report that Google crawler can’t render the page using 16 (link).
    • Fixed in 16.0.0-beta.3
  • Some popular third party libraries won’t work yet (e.g. Enzyme).
  • (Please report more issues in this thread.)

Updates

  • Release 16.0.0-beta.1 on 2017-07-24
  • Release 16.0.0-beta.2 on 2017-07-27
    • Fix a crash in server rendering.
  • Release 16.0.0-beta.3 on 2017-08-03
    • Fix strict-mode function scope problem (#10361)
    • Better error log messaging and cross-origin handling (#10353, #10373)
    • Shallow renderer improvements (#10372, #10342)
    • Edge-case context bugfix (#10334)
    • Single point of entry for server rendering (#10362)
    • etc.
  • Release 16.0.0-beta.4 on 2017-08-08
    • Warn about unresolved function as a child. #10376
    • Remove data-reactid and data-react-checksum from whitelist. #10402
    • New test renderer API features (disabled via feature flag, for now). #10377
    • And some other minor updates to slightly reduce bundle size.
  • Release 16.0.0-beta.5 on 2017-08-08
    • Fix bugs related to unmounting error boundaries #10403
    • Enable new fiber ReactTestRenderer API methods; (previously disabled behind a feature flag) #10410

dmitrif commented Jul 26, 2017 edited

In regards to:

There is no react/lib/* and react-dom/lib/* anymore. Even in CommonJS environments, React and ReactDOM are precompiled to single files ("flat bundles"). If you previously relied on undocumented React internals, and they don't work anymore, let us know about your specific case in this issue, and we'll try to figure out a migration strategy for you.

We have been relying on https://github.com/electrode-io/electrode-react-ssr-caching which monkey-patches the React render. Any suggestions as to how to make it work with v16?

Used imports:

const ReactCompositeComponent = require("react-dom/lib/ReactCompositeComponent");
const DOMPropertyOperations = require("react-dom/lib/DOMPropertyOperations");

Thank you.

Collaborator

aweary commented Jul 26, 2017

Maybe @aickin can comment on the feasibility of integrating a caching solution directly into the server renderer?

This initial React 16.0 release is mostly focused on compatibility with existing apps. It does not enable asynchronous rendering yet. We will introduce an opt-in to the async mode later during React 16.x.

Is there any way to manually enable asynchronous rendering in this beta?

Contributor

TrySound commented Jul 26, 2017

@verkholantsev Afaik there is an option. Or will be.

@TrySound Could you tell more about this option? Or could you tell me in what direction should I start my research about it?

Contributor

bvaughn commented Jul 26, 2017

@verkholantsev We'll provide info about enabling async once we think it's ready for testing. We're still experimenting on it internally.

LestaD commented Jul 26, 2017

Can I import SynteticEvent from flat bundle?

@clarkbw clarkbw referenced this issue in devtools-html/devtools-core Jul 26, 2017

Open

[WIP] Trying out react@next #525

kadikraman commented Jul 26, 2017 edited

This might be interesting: just tried it in our codebase. Using yarn to install the package, I get an error in HMR and from one of our dependencies, but using npm (I'm on 5.2.0), it works! 🎉

Member

gaearon commented Jul 26, 2017 edited

Can I import SynteticEvent from flat bundle?

Not currently. What is your use case for it?

Using yarn to install the package, I get an error in HMR and from one of our dependencies, but using npm (I'm on 5.2.0), it works!

We’ll need more details to tell if something is wrong. I’d appreciate if you could provide a reproducing project (even if it only fails with Yarn).

LestaD commented Jul 26, 2017 edited

@gaearon

Not currently. What is your use case for it?

Ex.: I want to pass change event to parent when toggle button.
Or I want to modify event before pass to parent.

// pseudocode
onClickItem(event) {
  const newEvent = new SynteticEvent('change', { id: this.state.currentId })
  this.props.onChange(newEvent)
}

So far, everything runs smoothly save for my test cases of rendered components, because they depend on:

import { mount, shallow } from 'enzyme';

which results in dependency errors like this:

react-dom is an implicit dependency in order to support react@0.13-14. Please add the appropriate version to your devDependencies. See https://github.com/airbnb/enzyme#installation

This is an app that was built from CRA - it is not ejected. This appears to be the only place in which we use enzyme (tests are run with react-scripts test --env=jsdom --coverage). Is there an equivalent of mount and shallow within Jest that I should be using?

when react enable me to call API with any built in package like $http in angular, without force me to use staff like superagent or axios ..

@muhammedMoussa : that is not a goal for React, and React will not be shipping a built-in HTTP client library.

Member

gaearon commented Jul 26, 2017

Let’s keep this discussion focused on 16 Beta. Thanks.

Contributor

mridgway commented Jul 26, 2017

A compiled list of all deprecations that have been removed from 15 -> 16 would be helpful as a checklist for upgrading.

Related: is the warning for unknown attributes (via the whitelist) still in place or will upgrading to 16 start adding them to the DOM without warning?

Member

gaearon commented Jul 26, 2017 edited

A compiled list of all deprecations that have been removed from 15 -> 16 would be helpful as a checklist for upgrading.

I think it’s React.createClass, React.PropTypes, React.__spread, React.createMixin, and React.DOM.*. There are a few changes to entry points too (e.g. shallow renderer is now react-test-renderer/shallow, and test utils moved to react-dom/test-utils). I believe they all are mentioned in these blog posts:

Related: is the warning for unknown attributes (via the whitelist) still in place or will upgrading to 16 start adding them to the DOM without warning?

This is sort of up in the air right now. I think we’ll make a decision before shipping next beta.

Contributor

wmertens commented Jul 26, 2017

Do the flat builds not interfere with scope hoisting by the app bundlers? Just wondering, not sure if there is much to gain from that. I presume the bundler would have to figure out statically that there are parts of React that are not used by the app and can be removed, probably a very tall order.

timdorr commented Jul 27, 2017

@wmertens The flat bundle is scope hoisted itself. You can continue to hoist it yourself. It's basically just concatenating the modules into one file and ensuring no variable name overlaps. That's fully compatible with any bundler above it.

Collaborator

aweary commented Jul 27, 2017

This is an app that was built from CRA - it is not ejected. This appears to be the only place in which we use enzyme (tests are run with react-scripts test --env=jsdom --coverage). Is there an equivalent of mount and shallow within Jest that I should be using?

For what it's worth, Enzyme will be supporting React 16 just like previous releases. There's a huge refactor underway which includes support for React 16.

Our large application ran perfectly except for our unit tests that depend on enzyme. This looks like our biggest blocker at this point.

7rulnik commented Jul 27, 2017 edited

Seems that react-dom/server doesn't work correctly with style in production mode . In dev all works fine.

const React = require('react')
const ReactServer = require('react-dom/server')
const foo1 = React.createElement("div", {
  className: "sign-link",
  id: "sign-layout-link",
  style: { position: 'absolute' }
});

ReactServer.renderToString(foo1)
TypeError: re is not a function
    at /Users/v7rulnik/projects/kupibilet/internals/kupibilet.ru/node_modules/react-dom/cjs/react-dom-server.production.min.js:1:7774
    at /Users/v7rulnik/projects/kupibilet/internals/kupibilet.ru/node_modules/fbjs/lib/memoizeStringOnly.js:23:32
    at f (/Users/v7rulnik/projects/kupibilet/internals/kupibilet.ru/node_modules/react-dom/cjs/react-dom-server.production.min.js:1:2423)
    at b (/Users/v7rulnik/projects/kupibilet/internals/kupibilet.ru/node_modules/react-dom/cjs/react-dom-server.production.min.js:1:3163)
    at e.renderDOM (/Users/v7rulnik/projects/kupibilet/internals/kupibilet.ru/node_modules/react-dom/cjs/react-dom-server.production.min.js:1:9665)
    at e.render (/Users/v7rulnik/projects/kupibilet/internals/kupibilet.ru/node_modules/react-dom/cjs/react-dom-server.production.min.js:1:8762)
    at e.read (/Users/v7rulnik/projects/kupibilet/internals/kupibilet.ru/node_modules/react-dom/cjs/react-dom-server.production.min.js:1:8452)
    at Object.T [as renderToString] (/Users/v7rulnik/projects/kupibilet/internals/kupibilet.ru/node_modules/react-dom/cjs/react-dom-server.production.min.js:1:4384)
    at repl:1:13
    at ContextifyScript.Script.runInThisContext (vm.js:44:33)

falconmick commented Jul 27, 2017 edited

The server renderer doesn't yet support returning arrays and strings from components.

Does this mean doesn't support them at all? or just as the root component i.e. ssr(<somecom /<<otherComp /<)?

This could be really bad if it does break it as array returns are life, but so is SSR

Contributor

bvaughn commented Jul 27, 2017 edited by spicyj

Our large application ran perfectly except for our unit tests that depend on enzyme. This looks like our biggest blocker at this point.

Flarnie and I spoke with Leland about Enzyme this afternoon. Seems like it's in a good spot to be 16-compatible by or before the 16.0 release and we'll be staying in touch about it during the beta.

Member

Daniel15 commented Jul 27, 2017

The server renderer has been completely rewritten, and now offers a streaming mode

Is the streaming mode specific to Node.js (ie. does it use APIs only available in Node), or could it work in other environments too (for example, for people using ReactJS.NET, react-rails or React in Java)?

@twobit twobit referenced this issue in lavrton/react-konva Jul 27, 2017

Closed

Not compatible with new version of React (15.4.0) #44

Contributor

jlongster commented Jul 27, 2017

This is an error I hit in a moderately sized app.

screen shot 2017-07-26 at 11 12 53 pm

It points to this code:

screen shot 2017-07-26 at 11 13 27 pm

If I open the Object.renderPortal frame it looks like it's in the react-modal library, these lines exactly.

At the very top of that file is does const renderSubtreeIntoContainer = ReactDOM.unstable_renderSubtreeIntoContainer; so this is an unstable API. I can look more into this tomorrow because it very well might be a false positive. I did update both react and react-dom to 16.0.0-beta.1.

I will look more into this tomorrow and see if I can provide more details.

Great work y'all! ❤️

Member

spicyj commented Jul 27, 2017 edited

@jlongster The code in your screenshot isn't 16 beta 1, but some of the stack frames you cited are. You must have two copies of react-dom, one new and one old. npm ls might help you find why.

Contributor

jlongster commented Jul 27, 2017

Ah ok, thanks! I will check tomorrow, not at my computer anymore

@miksansegundo miksansegundo referenced this issue in glenjamin/skin-deep Jul 27, 2017

Closed

Support for React 16 beta #91

Collaborator

aickin commented Jul 27, 2017

Maybe @aickin can comment on the feasibility of integrating a caching solution directly into the server renderer?

@dmitrif While I think it's certainly conceivable to implement a caching solution in the server renderer, I don't think it's amenable to monkey patching; the renderer is basically all just in one file. If it's really important to you, I'd fork the code (and hopefully consider contributing back!).

It's also worth noting that in some preliminary tests I did, the 16 renderer is 2-2.5x faster than the 15 renderer, so it's possible that you may not have as much of a need for caching.

Collaborator

aickin commented Jul 27, 2017

Does this mean doesn't support them at all? or just as the root component i.e. ssr(<somecom /<<otherComp /<)?

@falconmick I believe it doesn't support them anywhere in the tree right now, but it's a "known issue", which I believe means it should get fixed before release. (Anyone from core team, please feel free to correct me!)

Contributor

bvaughn commented Jul 27, 2017

@Daniel15 I think /server is browser-only APIs and /node-stream is browser+node APIs. The naming of these entry points isn't great and may change before final.

Member

spicyj commented Jul 27, 2017

@aickin @falconmick That's the plan! If anyone wants to work on adding that, we'd happily review a PR.

Collaborator

aickin commented Jul 27, 2017

Is the streaming mode specific to Node.js (ie. does it use APIs only available in Node), or could it work in other environments too (for example, for people using ReactJS.NET, react-rails or React in Java)?

@Daniel15 The streaming renderer does indeed depend on the Readable API in Node. For more info on the streaming API, check out its documentation.

However, I should note that the underlying code for the renderer is a function that will output N bytes at a time, which you could in theory call from just about any stream API. If you want to see code that uses this function, check out the implementation of ReactMarkupReadableStream. This is a proceed-at-your-own-risk kind of thing, though, because the internal class (ReactPartialRenderer) is not a supported API and could change at any moment.

Collaborator

aickin commented Jul 27, 2017

@7rulnik Great bug, and I'm able to repro on my machine. I've filed it as #10299. Thanks!

Can I try out React 16 beta with fiber in a react-native project?

Contributor

bvaughn commented Jul 27, 2017

@aakashbapna Instructions for that will be coming soon. Currently I think you'd have to build from source to try it.

@daochouwangu daochouwangu referenced this issue in daochouwangu/webfrontdaily Jul 27, 2017

Open

2017-07-27 前端日报 #74

Also seeing @7rulnik's issue #10294 (comment) regarding style on the server in production mode.

(re is not a function)

Hi! i opened an issue on react-virtualized repo #762

image

Thanks!

Member

gaearon commented Jul 27, 2017

Also seeing @7rulnik's issue #10294 (comment) regarding style on the server in production mode.

Fixed by #10300. Will go out in next beta after we catch a few more issues.

I believe it doesn't support them anywhere in the tree right now, but it's a "known issue", which I believe means it should get fixed before release.

There’s an open PR for this too: #10221

Can I try out React 16 beta with fiber in a react-native project?

Not yet. Please see the above post:

React Native follows a different release cycle and will update to the beta in a future release. (It's already using an alpha release but not yet using Fiber.)

leecade commented Jul 27, 2017

🔥

We've got some unit tests where we test that the data returned from a react-redux mapStateToProps and mapDispatchToProps conforms to the propTypes of the underlying component.

We do this through the use of react/lib/ReactPropTypesSecret so we can use the PropTypes testing function directly. Is there anything we could do moving forwards to replicate this functionality?

davidfurlong commented Jul 27, 2017 edited

Member

gaearon commented Jul 27, 2017

@TeaSeaLancs

We've got some unit tests where we test that the data returned from a react-redux mapStateToProps and mapDispatchToProps conforms to the propTypes of the underlying component.

You can do something like:

React.createElement(YourComponent, propsYouWantToTestWith);

This will trigger PropTypes validation. If there is a failure, it will use console.error() to output the warning. You can override console.error() in tests, and throw on any unexpected errors.

Fair enough but having to stub/monkey patch something as fundamental as console.error() leaves a bit of a sour taste in the mouth :\

Member

gaearon commented Jul 27, 2017 edited

Now that I think of it, actually it’s not necessary. PropTypes provides a first-class API for running checks. In fact it’s what React uses.

If you look at the warning message that you get when you call them directly, it says:

Calling PropTypes validators directly is not supported by the prop-types package. Use PropTypes.checkPropTypes() to call them.

As mentioned in prop-types README:

If you absolutely need to trigger the validation manually, call PropTypes.checkPropTypes(). Unlike the validators themselves, this function is safe to call in production, as it will be replaced by an empty function:

// Works with standalone PropTypes
PropTypes.checkPropTypes(MyComponent.propTypes, props, 'prop', 'MyComponent');

Hope this helps!

Contributor

jlongster commented Jul 27, 2017

@spicyj You were right, I had two versions of React. I made sure everything was on next and it's mostly working.

I still hit an error in one case. I use the react-modal library, and when try to close the modal it errors:

screen shot 2017-07-27 at 9 52 35 am

This is the line of code where the error is happening. For some reason it looks like this.portal is null.

Unfortunately, I created a small test case to try to reproduce, but that library works fine independently of my app. I don't know if this is 16's fault or not, but it definitely worked with 15. Sorry I can't reproduce in a small test case, but wanted to report this bug in case it gives more context into a possible bug.

@gaearon ... when did that get introduced? I swear I never saw that message :P

Member

gaearon commented Jul 27, 2017

@TeaSeaLancs I think during 15.5 or about that. You’re right it didn’t always exist, sorry!

Fair enough, we started on 15.2 originally.

Member

gaearon commented Jul 27, 2017

@jlongster Any chance you could put breakpoints around this.portal assignments and figure out why this.portal becomes null?

wbeard commented Jul 27, 2017 edited

All of my react/lib/* errors are from react-addons-* packages trying to use them. Are there beta releases of the addons libs?

Contributor

jlongster commented Jul 27, 2017

@gaearon Here's what I found. The renderSubtreeIntoContainer function that react-modal is calling here returns null. I stepped into that call and found where it is returning null. It's right here:

screen shot 2017-07-27 at 10 05 48 am

Here's the stack trace:

screen shot 2017-07-27 at 10 06 11 am

If it helps, here's what containerFiber is that it is checking to see if it has a child property:

screen shot 2017-07-27 at 10 08 07 am

So basically renderSubtreeIntoContainer is returning DOMRenderer.getPublicRootInstance(root) which returns null for some reason. I hope this isn't a false positive and I'm wasting your time...

jochenberger commented Jul 27, 2017 edited

@gaearon, @jlongster I bet it's caused by the changes to unstable_renderSubtreeIntoContainer (#5990, #9808)
Oh, to slow. ;-)

Member

gaearon commented Jul 27, 2017

@wbeard

All of my react/lib/* problems are from react-addons-* packages trying to use them. Are there beta releases of the addons libs?

All react-addons-* packages that are compatible with React 16 already have versions that don’t use internals (I think it’s 15.6.x). Try to update them. Some addons are incompatible with React 16 (e.g. -perf). You’d need to stop using them.

Member

gaearon commented Jul 27, 2017 edited

@jochenberger I don’t think those issues are related because this code has been rewritten from scratch. So past issues probably won’t apply.

@jlongster Thanks, this is useful. Let me take a look and I’ll probably have to ask for more info and screenshots.

I want to mention that longer term, unstable_renderIntoContainer is going away, and unstable_createPortal takes its place. It solves the timing issues. But I’d like to understand if what you encounter is a bug or not. It’s not clear to me yet.

Contributor

jlongster commented Jul 27, 2017

@gaearon If think this is an actual bug and ever want to even remotely control my machine to take a look, I'd be happy to let you :)

wbeard commented Jul 27, 2017

@gaearon Thanks! Are those addons temporarily incompatible or will they be deprecated?

Member

gaearon commented Jul 27, 2017

@wbeard Which addons specifically? There’s some info on #9207 but happy to answer more specific questions in this thread.

@jlongster Yes, that would be great. Are you free now? You can hop into my DMs.

wbeard commented Jul 27, 2017

@gaearon Sorry, should have been more precise. I was asking about -perf. I'm using the following addons:

"react-addons-create-fragment": "15.6.x",
"react-addons-css-transition-group": "15.6.x",
"react-addons-perf": "15.4.2",
"react-addons-pure-render-mixin": "15.6.x",
"react-addons-shallow-compare": "15.6.x",

I'll read over that issue to get me up to speed and see what needs to be cut out for now.

Member

gaearon commented Jul 27, 2017

Yes, you’d have to stop using -perf for now. The rest should keep working. We don’t have a plan for -perf yet, and likely won’t have it until some time during 16.x.

wbeard commented Jul 27, 2017

No worries, it's being used in a dev-only middleware, so we can come up with a different strategy most likely using marks.

Member

gaearon commented Jul 27, 2017 edited

@wbeard FWIW React 16 beta always emits performance.mark() and performance.measure() calls in DEV so you can capture those using Performance Observer API. See this proof of concept.

wbeard commented Jul 27, 2017

@gaearon Do you think that behavior will continue to be in future releases of 16?

Member

gaearon commented Jul 27, 2017

@wbeard What behavior?

wbeard commented Jul 27, 2017

@gaearon

always emits performance.mark() and performance.measure() calls in DEV

Member

gaearon commented Jul 27, 2017

I don’t know. We’ll start doing this for now. If people get inconvenienced we can add an opt-out API. But I’d like to try to keep it on by default.

@spyworldxp spyworldxp referenced this issue in FormidableLabs/nuka-carousel Jul 27, 2017

Open

createClass deprecated in React Fiber #230

zyzski commented Jul 27, 2017 edited

proxyConsole.js:56 Warning: The tag <nagivation> is unrecognized in this browser. If you meant to render a React component, start its name with an uppercase letter.

There is no navigation tag in my project, could it be possibly referencing a different react element i didn't uppercase? Can't seem to find where this is coming from. Just dropped react@next in my create-react-app.

Member

gaearon commented Jul 27, 2017

@zyzski You can open React DevTools and use search to find <navigation> in it. Indeed, there is no such HTML element. (Maybe they intended to use <nav>.)

Member

gaearon commented Jul 27, 2017

@LestaD

Ex.: I want to pass change event to parent when toggle button.

What’t the advantage of using SyntheticEvent here? In fact I would recommend to not pass event objects around because they get pooled. Read the data early, and pass a plain object representing the data you care about.

@jeremyadavis jeremyadavis referenced this issue in react-boilerplate/react-boilerplate Jul 27, 2017

Open

feat(core) add react performance profiling by default #1635

Member

gaearon commented Jul 27, 2017 edited

Update: Just published 16.0.0-beta.2 which fixes the SSR crash.
Please give SSR another test round!

Contributor

mjackson commented Jul 31, 2017

The extra work that is required here to get the React 16 beta on cdnjs is exactly why I created unpkg in the first place. It's zero config. This let's package authors focus on improving their libraries instead of configuring CDNs.

unpkg currently serves more than 8M requests per day for React in particular, so you can be sure the cache is always hot.

As for SRI support, we will get it added as soon as possible. This is the first time anyone's asked for it.

Member

Daniel15 commented Jul 31, 2017 edited

@mjackson

The extra work that is required here to get the React 16 beta on cdnjs

There's no extra work other than a once-off configuration that someone needs to do (update CDNJS to load React from npm rather than from the Bower repo). Once that's configured, CDNJS automatically grabs the packages from npm as soon as they're published. 👍

Contributor

mjackson commented Jul 31, 2017

There's no extra work other than a once-off configuration that someone needs to do

Yep, that's the only thing I was referring to. As someone who publishes a lot of packages to npm, that's the work I was trying to avoid with unpkg 😅

virgofx commented Jul 31, 2017 edited

@mjackson Unpkg has definitely been useful and very convenient. Appreciate the effort in helping out the community. I do know of a bunch of other Github threads that do mention not having SRI over the last year and that was a concern for a lot to transition to CDNJS. Regardless, the backend is still CloudFlare so it really doesn't matter in the end. Thus, from a technical standpoint ... the only differences being:

CDNJS Unpkg
Provider CloudFlare CloudFlare
Modification Header etag last-modified
Cache Expiration 355 days 365 days
SRI Support (API + Web)
STS Support
X-Content-Type
Extra Headers via

Note: Yes, SRI only needs API support and Web UI support. No other backend changes. This is to allow buid/CI to fetch values to be added into projects HTML/asset refs.

I'm in processing of updating config for CDNJS so both options will be available. If React team wants to move documentation reference ... that's up to them.

@styfle styfle referenced this issue in unpkg/unpkg-website Jul 31, 2017

Closed

Add support for Subresource Integrity (SRI) #48

Contributor

mjackson commented Jul 31, 2017

@virgofx I read through the SRI spec this morning, but AFAICT unpkg doesn't actually need to do anything to "support SRI".

In SRI, the integrity of a resource is calculated by the browser to see if it matches the value of the integrity attribute on script and link elements. In other words, whether or not you use SRI is completely up to the client.

You can use the following script tags to load React 16.0.0-beta.2 from unpkg right now, using SRI:

<script src="https://unpkg.com/react@16.0.0-beta.2/umd/react.production.min.js"
  integrity="sha384-lJJbYsDe7wDuOttI/MIHfj68o3fVZOhJDxEn0cTPbDq5mkzjF1p+AFEp3r/HpUnt"
  crossorigin="anonymous"></script>
<script src="https://unpkg.com/react-dom@16.0.0-beta.2/umd/react-dom.production.min.js"
  integrity="sha384-urrH8a5NtX87DSmFO4pKPICl5RHszNM6yx75JIYhynT2ukWDK4skf0YAh4EDEdD2"
  crossorigin="anonymous"></script>

You can also see these in use in a working demo.

One thing I guess we could do to make this easier for people would be to include ready-made script tags that already include the SRI value so people don't have to calculate it themselves, but nothing is preventing the React website from including the integrity attribute on their script tags that use unpkg.

Member

gaearon commented Jul 31, 2017 edited

Let’s keep the future comments focused on React beta.
(This was an interesting discussion though! Happy to continue in another issue.)

arkwright commented Jul 31, 2017 edited

We are particularly interested in hearing about performance differences you notice between 15.x and 16.x. We don't expect any massive changes but would love to know about improvements or regressions. Please report them here!

Just finished installing react@16.0.0-beta.2 and I'm seeing a 20–25% reduction in server-side rendering response times for our app, depending on the page. Thanks for all of your hard work! 🎉

virgofx commented Aug 1, 2017

Similarly ... No official benchmarks but purely swapping out 15.6.1 for 16.0.0-beta.2 is resulting in page load times about 20-30% faster. That's combined effects of both server side rendering and client-side.

Is there any way to enable async rendering manually, now?

Member

gaearon commented Aug 1, 2017 edited

@kononenko-a-m There technically is, but it’s not ready, and is missing much of the logic that would make it work well. So let’s say there isn’t.

@gaearon I know it's too early to ask, but maybe you can tell when we could expect it? at least available for some experimental purpose

Member

gaearon commented Aug 1, 2017

Some time this year hopefully.

thanks a lot! will follow the updates :) I'm very excited to try this technology!

Contributor

koba04 commented Aug 1, 2017

@bvaughn

Shallow renderer doesn't implement unstable_batchedUpdates. (TBD)

I'm not sure why ShallowRenderer has to implement unstable_batchedUpdates.
Does the issue mean that ShallowRenderer should support React(DOM|Native)._unstable_batchedUpdates or implement ShallowRenderer.unstable_batchedUpdates?

If it means the latter, I'm not sure its use case.
AFAIK, ReactDOMServer doesn't implement it.

Member

gaearon commented Aug 1, 2017

@koba04 We bumped into this because Enzyme relies on unstable_batchedUpdates internally. We haven’t investigated why, or whether we want to support this, yet.

Contributor

koba04 commented Aug 1, 2017

@gaearon Thanks, that makes sense. it was my work😅

I've added it to emulate batchedUpdates in lifecycle methods and event handlers.

Contributor

bvaughn commented Aug 1, 2017

Thanks, that makes sense. it was my work😅

@koba04 You might be in a better place than we are at the moment then in terms of deciding whether it's worth adding batched updates support to the shallow renderer. 😁

The team chatted about it briefly during our weekly sync yesterday, but the resolution was just to try and learn more about the required use cases for it, and determine if there were easier solutions.

andy-j-d commented Aug 1, 2017

@jlongster @gaearon were you able to resolve the modal issue? I'm getting a similar(?) error using react-bootstrap modals.

Error:

Uncaught TypeError: Cannot read property 'contains' of undefined
    at eval (contains.js?64f5:17)
    at Modal.focus (Modal.js?39f5:557)
    at Modal.onShow (Modal.js?39f5:486)
    at Modal.componentDidUpdate (Modal.js?39f5:243)
    at commitLifeCycles (react-dom.development.js?cada:10893)
    at commitAllLifeCycles (react-dom.development.js?cada:11665)
    at HTMLUnknownElement.callCallback (react-dom.development.js?cada:1303)
    at Object.invokeGuardedCallbackDev (react-dom.development.js?cada:1338)
    at invokeGuardedCallback (react-dom.development.js?cada:1199)
    at commitAllWork (react-dom.development.js?cada:11786)

The error is in node_modules/dom-helpers/query/contains.js?64f5

This is the block that fails - context is undefined

exports.default = function () {
  // HTML DOM and SVG DOM may have different support levels,
  // so we need to check on context instead of a document root element.
  return _inDOM2.default ? function (context, node) {
    if (context.contains) {
      return context.contains(node);
    } else if (context.compareDocumentPosition) {
      return context === node || !!(context.compareDocumentPosition(node) & 16);
    } else {
      return fallback(context, node);
    }
  } : fallback;
}();

tomasztomys commented Aug 1, 2017 edited

React-modal doesnt't work too @gaearon @andy-j-d

tolmasky commented Aug 1, 2017

We use react-dom/lib/EventPluginUtils. Specifically, we monkey patch executeDispatchesInOrder to see if the model changed in an event handler since we use a single state atom, something like this:

executeDispatchesInOrder = function() { originalExecuteDispatchesInOrder(); if (change) rerender() }

Anyway we can still get access to this?

Member

gaearon commented Aug 1, 2017

@tomasztomys @andy-j-d

Modal issues might be related to #10309. Please see above:

  • ReactDOM.render() and ReactDOM.unstable_renderIntoContainer() now return null if called from inside a lifecycle method.
  • To work around this, you can either use the new portal API or refs.

If something else is causing this, please create a small reproducing example we can run, and we’ll take a look.

Member

gaearon commented Aug 1, 2017

@tolmasky I don’t think we’ll be exposing the event system, but I think people wanted to wrap all event handlers for some other purposes (e.g. error handling). Can you please open a new issue with more details about your use case?

Member

sebmarkbage commented Aug 1, 2017

@tolmasky I'm curious if you could wrap addEventListener in the DOM instead? Also curious if you could use requestAnimationFrame or requestIdleCallback to ensure that you're batched up across async cycles. Let's discuss in an another issue.

hallaathrad commented Aug 1, 2017 edited

Is anyone else getting this from react-dom?

Uncaught SyntaxError: In strict mode code, functions can only be declared at top level or immediately within another function.

Seems like using 'strict mode' makes the browser --chrome 40. Client requirement. I know-- not like having 'function recomputePluginOrdering()' on line 53 be inside of an 'if' statement (or any such block). Odd enough, newer browsers seem to ignore the "second" part here...

Any clues on a workaround for older browsers?

Contributor

koba04 commented Aug 2, 2017

@bvaughn

The team chatted about it briefly during our weekly sync yesterday, but the resolution was just to try and learn more about the required use cases for it, and determine if there were easier solutions.

That makes sense.
Let me know if you need some help, I'm starting to investigate this. Let's discuss in an another issue.

The following is a rough implementation of unstable_batchedUpdates for ShallowRenderer.

koba04/react@ca78c92

Contributor

bvaughn commented Aug 2, 2017 edited

Regarding @hallaathrad's comment about the "use strict" error in older Chrome versions, I've verified it in Chromium <= 41. (Chromium 43+ does not show this error. Unfortunately I'm unable to get any of the Chromium 42 versions to run locally without insta-crashing I'm not sure how it behaves.)

There seem to be a couple of ways we could address this:

  • Remove the outer if-DEV condition. (We removed this check from React Native as well to address problems with the Android JSC; see #10171.)
  • Remove the "use strict" directive from the bundles.
  • Use a Rollup plugin to convert the floating functions from function foo() {} to var foo = function() {}.
  • Wrap the contents of the if-DEV conditional in an IIFE.

I believe outer if-DEV conditional was added to help with weak dead code elimination- presumably in case projects accidentally included both dev and production builds of react/react-dom. (See #9969) cc @gaearon, @trueadm to verify and share any thoughts on this.

cc @sebmarkbage

Member

gaearon commented Aug 2, 2017

Wrap the contents of the if-DEV conditional in an IIFE.

This would make sense to me as a solution.

These, coming from a very respectful, and humbly uninformed point of view:

Use a Rollup plugin to convert the floating functions from function foo() {} to var foo = function() {}.

@bvaughn Seems to me if the idea was to have that if-DEV for testing purposes, this would be the most backwards compatible, "legal" workaround (if keeping strict mode, anyway). Should be good enough at least in these Beta stages.

@gaearon Might I enquire why you prefer that option?


I'm still just surprised to find out the current browser versions have decided to outright ignore that part of the specs, though.

Member

gaearon commented Aug 2, 2017

@hallaathrad

Might I enquire why you prefer that option?

It’s very easy to do on string level without messing with AST.

Contributor

bvaughn commented Aug 3, 2017 edited by gaearon

Just a quick note that 16.0.0-beta.3 was just released with a couple of improvements and bug-fixes to things reported above in this thread, including:

  • Fix strict-mode function scope problem (#10361)
  • Better error log messaging and cross-origin handling (#10353, #10373)
  • Shallow renderer improvements (#10372, #10342)
  • Edge-case context bugfix (#10334)
  • Single point of entry for server rendering (#10362)
  • Support for returning strings and arrays in server render (#10221)
  • No data-reactid attributes in the server markup anymore (#10339)

Like before, install it with:

yarn add react@next react-dom@next

Or:

npm install --save react@next react-dom@next

The list of breaking changes in the top post has been updated.

Contributor

Andarist commented Aug 4, 2017

Map+Set core-js' polyfills weigh 5kB gzipped. Not the end of the world, but would be gr8 if you specify what exact features of those built-ins do u expect - besides probably the basic interface.

core-js follows spec really closely and probably some 'loose'/smaller equivalents could be created.

Member

gaearon commented Aug 4, 2017

what exact features of those built-ins do u expect

It’s a bit tricky to give specifics because this limits our ability to change the code. But this should give you an idea (these are not all methods we use, but the ones that differ from “basic support” according to MDN).

Contributor

Andarist commented Aug 4, 2017

Simpler polyfill could be developed alongside react and tests could be run with it, that way it wouldnt limit you. I might actually do exactly that, just need to configure this thing properly to run with your test suite.

@oliviertassinari oliviertassinari referenced this issue in reactjs/react-transition-group Aug 5, 2017

Open

React 16 beta issue #151

Contributor

oliviertassinari commented Aug 5, 2017

@tomasztomys @andy-j-d

Material-UI use a custom fork of the Modal of react-bootstrap. We have been facing that issue too. We ended-up using the new portal API of React over the old unstable_renderIntoContainer. You can find the abstraction in our Portal.js component.

coldi commented Aug 6, 2017 edited

How do I get access to ReactFiberReconciler in 16 beta?

This doesn't work anymore because of the removed react-dom/lib/*:
https://github.com/reactjs/react-art/blob/master/src/ReactARTFiber.js

Member

gaearon commented Aug 6, 2017 edited

@coldi There is no way in a beta, but we’re tracking exposing it in some way in #9103.
react-art is currently published from this repo (and available with @next tag on npm).

coldi commented Aug 6, 2017

thanks dan for clearification. it's not that i need react-art. i was only looking into it to learn more about custom renderer in fiber and i would be really happy if you could make it available for public in react 16 soon-ish :) it's an awesome feature i'm looking forward to.
anyway, thanks guys for your great support.

@gaearon, looks like Gajus0 confirmed that with beta 3, the Google bug is not there anymore: https://www.reddit.com/r/javascript/comments/6pqrz5/react_16_beta/dkrgvzj/

Member

gaearon commented Aug 6, 2017

Thanks for letting us know!

HriBB commented Aug 6, 2017 edited

@coldi I use a fork to export ReactFiberReconciler through react-dom

--- a/src/renderers/dom/fiber/ReactDOMFiberEntry.js
+++ b/src/renderers/dom/fiber/ReactDOMFiberEntry.js
@@ -729,6 +729,8 @@ var ReactDOMFiber = {
 
   findDOMNode: findDOMNode,
 
+  ReactFiberReconciler: ReactFiberReconciler,
+
   unstable_createPortal(
     children: ReactNodeList,
     container: DOMContainer,

HriBB commented Aug 6, 2017

@gaearon this might be a really stupid question, but how can we return array of elements from render()? I keep getting this error

ERROR in ./src/components/Home/View.js
Module build failed: SyntaxError: Adjacent JSX elements must be wrapped in an enclosing tag (30:4)

  28 |   return [
  29 |     <h1>Test</h1>
> 30 |     <p>This should work with React 16</p>
     |     ^
  31 |   ]
  32 | }
  33 | 

 @ ./src/components/Home/Container.js 33:12-43
 @ ./src/components/App/View.js
 @ ./src/components/App/Container.js
 @ ./src/components/Root/Container.js
 @ ./src/index.js
 @ multi react-hot-loader/patch webpack-dev-server/client?http://0.0.0.0:3000 webpack/hot/only-dev-server ./src/index
webpack: Failed to compile.

Is there a config option for babel or ...?

Member

gaearon commented Aug 6, 2017 edited

@HriBB I think you forgot a comma (since these are array items, regular JS rules apply).

return [
  <div key="1" />, // note the comma
  <div key="2" />
];

In the future we might consider adding first-class support for fragments in JSX that could look like:

return (
  <>
    <div />
    <div />
  </>
);

This is not related to React per se though, so it’s not blocking 16 release.

Based on that example (@gaearon), do arrays returned from render require keyed elements? Having played around with the beta, I don't think I get warnings if I omit them.

Member

gaearon commented Aug 7, 2017

Yes, they need keys.
If we had a special syntax like I mentioned above, we wouldn’t need to, but we don’t at the moment.

Contributor

bvaughn commented Aug 8, 2017

Just a quick note that 16.0.0-beta.4 was just released with a couple of small improvements:

  • Warn about unresolved function as a child. #10376
  • Remove data-reactid and data-react-checksum from whitelist. #10402
  • New test renderer API features (disabled via feature flag, for now). #10377
  • And some other minor updates to slightly reduce bundle size.

Like before, install it with:

yarn add react@next react-dom@next

Or:

npm install --save react@next react-dom@next
Contributor

bvaughn commented Aug 8, 2017

After some discussion we decided to enable the new react-test-renderer APIs too. They're available in16.0.0-beta.5 ~ "mo betas, mo betta" 😎

  • Fix bugs related to unmounting error boundaries #10403
  • Enable new fiber ReactTestRenderer API methods; (previously disabled behind a feature flag) #10410

Like before, install it with:

yarn add react@next react-dom@next

Or:

npm install --save react@next react-dom@next

Some popular third party libraries won’t work yet (e.g. Enzyme).

Is this the last major obstacle blocking the release?

Contributor

bvaughn commented Aug 8, 2017

Some popular third party libraries won’t work yet (e.g. Enzyme).

Is this the last major obstacle blocking the release?

It isn't exactly a release blocker. But Enzyme also seems to be pretty close to 16-compatibility, based on a recent chat between @flarnie, @lelandrichardson, and I.

@mastilver mastilver referenced this issue in FormidableLabs/rapscallion Aug 10, 2017

Open

Plan to migrate to react@16? #114

irom-io commented Aug 10, 2017 edited

2017-08-11_00-33-16
I have warnings for migrate to React 16.

In my own code i replace all usages PropTypes and CreateClass to React 16 format.

It is warnings from npm packages.
How can i understand in which modules needs changes?

andy-j-d commented Aug 10, 2017 edited

@irom-io Hit the little caret next to "Warning" to view the stack, you should be able to tell which library is causing the warning there. Many actively maintained libraries have been updated to fix those deprecations, you might need to fork/fix/PR if not.

Edit: what @bvaughn said haha.
PS: cc @bvaughn as far as I know this warning will only show one violation at a time, so if you have say ten libraries causing the same warning, you'll have to find them one by one after fixing each.

Contributor

bvaughn commented Aug 10, 2017

Click the little triangle to the left of "Warning" to expand it and see the stack trace. This will tell you which NPM module (presumably) is triggering the warning.

Here's a screencap of what I'm describing:
https://twitter.com/brian_d_vaughn/status/851110928720445443

irom-io commented Aug 10, 2017

2017-08-11_01-31-59

@bvaughn, @andy-j-d
Hm, i see only this: printWarning | @ | lowPriorityWarning.js:40
ESLint from "react-scripts": "1.0.10"

Contributor

bvaughn commented Aug 10, 2017

Hm. You could try overriding console.warn and see what's calling it?

const warn = console.warn;
console.warn = (...args) => {
  warn(...args);
  debugger;
};
Member

gaearon commented Aug 10, 2017

I guess console.warn doesn't show trace after all? :-(

Contributor

bvaughn commented Aug 10, 2017

It does in my experience. Looks like it doesn't always though. ☹️

Are you using up-to-date Chrome @irom-io? Could be a browser thing.

irom-io commented Aug 10, 2017

2017-08-11_01-57-29
const warn = console.warn; console.warn = (...args) => { warn(...args); debugger; };

Insert it in index.js and warnings are shown.

irom-io commented Aug 10, 2017

@andy-j-d It is Opera 46.0 based on chrome.

irom-io commented Aug 10, 2017

@bvaughn , @andy-j-d , i can push example app with this (not collapsable) warnings, if you want to try it reproduce.

Contributor

bvaughn commented Aug 10, 2017

I'm not sure I understand what the problem is at this point. Looks like you're seeing a stack trace above?

irom-io commented Aug 10, 2017

Maybe i seeing a stack trace above, i don't know.
I create a simple app, based on my app: https://github.com/irom-io/react_test. And all errors are shown correct.
So, some problems in my code or in node modules in my project, which broken console.warn.

Contributor

bvaughn commented Aug 10, 2017 edited

Running irom-io/react_test locally (with Chrome) I see:
screen shot 2017-08-10 at 4 52 16 pm

The warning is coming from react-router. Looks like you're using version 3.0.2, which is older than this deprecation. I think they fixed the prop-types warning in version 4.1.1 3.0.4. Update?

React Router 3 has a fix in 3.0.4, current is 3.0.5.

irom-io commented Aug 11, 2017

@bvaughn, @andy-j-d, Thanks, i will update it) Update is not a problem, the problem was exactly what to update)

Contributor

bvaughn commented Aug 11, 2017

I guess we learned that Chrome does a better job of reporting the warning stack than Opera does 😅

Collaborator

aickin commented Aug 11, 2017 edited

FYI, I just added issue #10442 against the server renderer in 16 not respecting componentDidCatch.

ETA: @spicyj and @sebmarkbage helpfully let me know that this is intentional. Sorry for the distraction!

im getting this in this library https://github.com/maccuaa/formsy-react-2 which im attempting to fork and fix since it and its parent are dead but i have no idea how to begin to track down that error, seems a bit vague. could anyone give any tips on what would cause that?

Contributor

bvaughn commented Aug 18, 2017 edited

@th3fallen Looks like you're mixing react@15 with react-dom@16?

I'm guessing this based on the fact that ReactBaseClasses.js shows up in the stack trace you've shown above when I'd expect to see react.development.js.

@bvaughn augh you're right i have another lib that required it.. let me drop that and report back. thanks

yep that was exactly it facepalm thanks

Contributor

bvaughn commented Aug 18, 2017

No problem! 😁 Glad it was an easy fix.

shkrishms commented Aug 18, 2017 edited

Hi,
I am trying out the beta React 16 version and I am a little confused about the expected error handling behavior here:

<ErrorBoundary>
  <GoodComponent1/>
  <GoodComponent2>
        <ErrorComponent/>
</GoodComponent2>
<ErrorBoundary>

The componentDidCatch lifecycle hook is not fired if there is an error in children of children to ErrorBoundary i.e if there is an error in ErrorComponent, I see the component stacktraces, but the lifecyclehook (componentDidCatch) is not triggered in ErrorBoundary.

If there is an error in the direct children, then it seems to work.
Is this expected behavior?

ntucker commented Aug 19, 2017

@shkrishms did you intend to have a closing ErrorBoundary on your last line? i.e.,

<ErrorBoundary>
  <GoodComponent1/>
  <GoodComponent2>
        <ErrorComponent/>
  </GoodComponent2>
</ErrorBoundary>

yes. Sorry. It was a typo. But I did close it properly in my original code.

Contributor

bvaughn commented Aug 19, 2017

@shkrishms Error boundaries catch errors in any descendants (not just direct children).

Here's an example: https://jsfiddle.net/bvaughn/haznhghm/

shkrishms commented Aug 19, 2017 edited

Thanks for the clarification. When I tried to troubleshoot why it was not working for me, I found that itis because I have a flyout i.e I open a new panel/flyout inside the main component. I guess its because its a different DOM? The other cases it works as you said.

<ErrorBoundary>
  <GoodComponent1/>
  <GoodComponent2>
        <ErrorComponentFlyout/>
  </GoodComponent2>
</ErrorBoundary>
Contributor

bvaughn commented Aug 19, 2017

@shkrishms I think you're going to need to provide a repro. It's hard to speculate. 😄

@salmanm salmanm referenced this issue in glortho/react-keydown Aug 19, 2017

Open

sometime we end up with no events bound #66

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment