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

feat: Add option to render concurrent roots #507

Closed
wants to merge 5 commits into from

Conversation

eps1lon
Copy link
Member

@eps1lon eps1lon commented Oct 24, 2019

First draft. There's a lot to figure out (like error message if the experimental build isn't used or if this cleanup is correct etc)

What:

Adds root option to render which currently only allows concurrent but could be used to create blocking roots instead

Why:

hype-driven-development

How:

Added a lot of branching logic but overall the implementation would be simpler with create*Root.

Checklist:

  • Documentation added to the
    docs site
  • Tests
  • Typescript definitions updated
  • Ready to be merged

@eps1lon eps1lon added the enhancement New feature or request label Oct 24, 2019
@eps1lon eps1lon requested a review from kentcdodds October 24, 2019 18:54
@@ -1 +1,2 @@
import '@testing-library/jest-dom/extend-expect'
jest.mock('scheduler', () => require('scheduler/unstable_mock'))
Copy link
Member Author

@eps1lon eps1lon Oct 24, 2019

Choose a reason for hiding this comment

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

Biggest unknown to figure out. I think we need to use this to advance time (or something like that) if we have tests scheduling work (like setState).

@eps1lon eps1lon mentioned this pull request Oct 25, 2019
@danielkcz
Copy link
Contributor

I wonder why are tests failing to find createRoot method. The package.json is correctly updated, there is no lock file to update, so it's rather weird.

"react": "^16.9.0",
"react-dom": "^16.9.0",
"react": "^0.0.0-experimental-f6b8d31a7",
"react-dom": "^0.0.0-experimental-f6b8d31a7",
Copy link
Member

Choose a reason for hiding this comment

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

I think that this resolves to the latest stable version. If we want to support the experimental release we have to do an exact version rather than ^. I believe that's why tests are failing.

@kentcdodds
Copy link
Member

I'm not sure how close concurrent mode is (but I get the impression that it's not very close) so I think we should just close this for now and open it when concurrent mode is closer.

Thanks!

@kentcdodds kentcdodds closed this Mar 3, 2020
@eps1lon
Copy link
Member Author

eps1lon commented Mar 3, 2020

Yeah that was me playing around. Not sure how testing will look with a mocked scheduler.

@eps1lon eps1lon deleted the feat/concurrent branch March 3, 2020 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants