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

Upgrade to React 16 #1178

Merged
merged 2 commits into from
Oct 13, 2017
Merged

Upgrade to React 16 #1178

merged 2 commits into from
Oct 13, 2017

Conversation

renchap
Copy link
Contributor

@renchap renchap commented Sep 24, 2017

As React 16 (facebook/react#10294) will be released in the coming days, here is a PR about it.

First, it changes the peerDependencies for all packages to allow React 16 (RC and up), see https://twitter.com/sophiebits/status/906661088472813568 for reference.

Secondly, I changed the main package.json to use React 16. This makes the tests to run against React 16. The changes are only cosmetic:

  • React 16 no longer adds a trailing ; to style attributes
  • React 16 outputs 0 and not 0px for sizes in style

I can change the PR to not bump tests and React version if you prefer to continue developing for React 15. Ideally tests should be ran on both React 15 and React 16 for some time, but I do not really know the best way to achieve this.

@renchap
Copy link
Contributor Author

renchap commented Sep 24, 2017

Benchmarks when switching to React 16. Note that this initial release does not focus on performance and most of the new optimisations will be rolled out later.

  benchmarks
    html-serializer
      deserialize
        1.43 → 1.34 iterations/sec
      serialize
        50.38 → 148.29 iterations/sec (194% faster) 🙌
    plain-serializer
      deserialize
        63.17 → 63.00 iterations/sec
      serialize
        1274.05 → 1158.92 iterations/sec
    rendering
      normal
        7.29 → 17.21 iterations/sec (136% faster) 🙌
    changes
      delete-backward
        377.70 → 350.65 iterations/sec
      insert-text
        722.32 → 703.59 iterations/sec
      normalize
        773.34 → 843.78 iterations/sec
      split-block
        127.07 → 137.61 iterations/sec
    models
      from-json
        32.98 → 29.59 iterations/sec
      get-blocks-at-range
        120327.62 → 115503.42 iterations/sec
      get-blocks
        428580.82 → 419921.69 iterations/sec
      get-characters-at-range
        118790.06 → 114335.71 iterations/sec
      get-characters
        447948.87 → 420296.31 iterations/sec
      get-inlines-at-range
        119525.79 → 117127.73 iterations/sec
      get-inlines
        435317.31 → 436656.08 iterations/sec
      get-marks-at-range
        118955.68 → 117397.10 iterations/sec
      get-marks
        441303.28 → 427722.65 iterations/sec
      get-path
        133173.62 → 130954.10 iterations/sec
      get-ranges
        383013.99 → 380286.46 iterations/sec
      get-texts-at-range
        120308.49 → 115807.23 iterations/sec
      get-texts
        426360.55 → 422012.47 iterations/sec
      to-json
        365.57 → 321.67 iterations/sec
      update-node
        2243.88 → 2284.71 iterations/sec

@Nantris
Copy link
Contributor

Nantris commented Sep 24, 2017

Just discovered React-Portal does not yet work with React 16. There will be a native solution, but it's thus far undocumented. The Hovering Menu example will need to be updated by someone more skilled than myself.

facebook/react#10675

@doodlewind
Copy link

There are some known IME bugs caused by IME's default behavior, which breaks react's vDOM against DOM. Is it possible to utilize React 16's Error Boundary to fix them?
#1114 #854 #810 #967

@Nantris
Copy link
Contributor

Nantris commented Sep 27, 2017

<3 Error boundaries

@renchap
Copy link
Contributor Author

renchap commented Sep 27, 2017

React 16 is now released 🎉 https://facebook.github.io/react/blog/2017/09/26/react-v16.0.html
What would be needed to merge this PR?
Examples should be rewritten using the builtin portals, is it something else?

Error boundaries are very nice, but this would mean dropping compat with React 15. I am not sure this is a good idea right now, maybe lets see how fast projects are upgrading?

@renchap renchap changed the title WIP: Switch to React 16 Upgrade to React 16 Sep 28, 2017
@isubasti
Copy link
Contributor

isubasti commented Oct 9, 2017

any update on this?. would love to upgrade my project's to use react16 to get the performance improvement

@renchap
Copy link
Contributor Author

renchap commented Oct 9, 2017

I updated the PR to only allow React 16 final, and rebased against master.

I have one test failure locally, but I also have it on master:

  432 passing (4s)
  1 pending
  1 failing

  1) slate-react plugins core onKeyDown split-empty-block:
     Error: You must pass a `schema` object.
      at assertSchema (packages/slate/lib/changes/normalize.js:204:11)
      at Changes.normalizeNodeByKey (packages/slate/lib/changes/normalize.js:57:3)
      at Change.call (packages/slate/lib/models/change.js:180:10)
      at Change.(anonymous function) [as normalizeNodeByKey] (packages/slate/lib/models/change.js:254:15)
      at Changes.normalizeDocument (packages/slate/lib/changes/normalize.js:45:10)
      at Change.call (packages/slate/lib/models/change.js:180:10)
      at Change.(anonymous function) [as normalizeDocument] (packages/slate/lib/models/change.js:254:15)
      at Changes.normalize (packages/slate/lib/changes/normalize.js:31:10)
      at Change.call (packages/slate/lib/models/change.js:180:10)
      at Change.(anonymous function) [as normalize] (packages/slate/lib/models/change.js:254:15)
      at Object.onBeforeChange (packages/slate-react/src/plugins/core.js:56:12)
      at Stack.(anonymous function) [as onBeforeChange] (packages/slate/lib/models/stack.js:204:33)
      at Simulator.(anonymous function) [as keyDown] (packages/slate-simulator/lib/index.js:65:11)
      at exports.default (packages/slate-react/test/plugins/core/on-key-down/split-empty-block.js:6:13)
      at _callee$ (packages/slate-react/test/plugins/index.js:30:13)
      at tryCatch (node_modules/regenerator-runtime/runtime.js:64:40)
      at Generator.invoke [as _invoke] (node_modules/regenerator-runtime/runtime.js:299:22)
      at Generator.prototype.(anonymous function) [as next] (node_modules/regenerator-runtime/runtime.js:116:21)
      at step (packages/slate-react/test/plugins/index.js:27:191)
      at packages/slate-react/test/plugins/index.js:27:437
      at Promise (<anonymous>)
      at Context.<anonymous> (packages/slate-react/test/plugins/index.js:27:99)

@Nantris
Copy link
Contributor

Nantris commented Oct 9, 2017

@isubastiCadmus I'm using Slate with React 16 without any issues. It's just that some examples are not compatible, but as far as I know it's only floating menu, and it might be compatible now with the new react-portal package.

@renchap renchap force-pushed the react-16 branch 2 times, most recently from ee09b36 to f27a41d Compare October 13, 2017 21:45
Changes are cosmetic:
- React 16 no longer adds a trailing `;` to `style` attributes
- React 16 outputs `0` and not `0px` for sizes in `style`
@ianstormtaylor
Copy link
Owner

Thanks @renchap!

@ianstormtaylor ianstormtaylor merged commit e960918 into ianstormtaylor:master Oct 13, 2017
@renchap renchap deleted the react-16 branch October 13, 2017 22:01
z2devil pushed a commit to z2devil/slate that referenced this pull request Dec 6, 2024
* Allow React 16 in peerDeps and use it for dev

* Make tests pass with React 16

Changes are cosmetic:
- React 16 no longer adds a trailing `;` to `style` attributes
- React 16 outputs `0` and not `0px` for sizes in `style`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants