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

DIO 8 #28

Merged
merged 265 commits into from Oct 6, 2017
Merged

DIO 8 #28

merged 265 commits into from Oct 6, 2017

Conversation

thysultan
Copy link
Member

Ready for the most part, mostly tweaking the level of abstractions to allow for a clean transition into the 2-phase commit->flush approach for the next minor release before/when react fiber(async mode) is released and cooperative scheduling is proven a necessary abstraction.

Currently the infrastructure looks like reconcile -> commit -> renderer.
The transition would make it look like reconcile -> commit ... [DOM/Native...]renderer(commits).

- better handle children as props pattern
- always convert `tabIndex` attribute to lowercase in server render
- handle recursive error in error boundaries
- insure root node is passed to componentWillUnmount lifecycle
- more tests
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 8ae3e87 on V8 into ** on master**.

@thysultan
Copy link
Member Author

Agreed. I think we are finally close to a release. Did you find any other hiccups?

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 99f3672 on V8 into ** on master**.

@Zolmeister
Copy link

Zolmeister commented Oct 2, 2017

I'm working on a safari mobile diffing issue, but I say go ahead and cut a release-candidate.

Some high-level API notes:
I'd remove unmountComponentAtNode, seeing as it's equivalent to render(null, $el)
I'm weary of findDOMNode; it smells like a foot-gun (I don't see a good reason to use it over lifecycle methods, and even then direct DOM manipulation is generally a bad idea)

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 05abc71 on V8 into ** on master**.

@thysultan
Copy link
Member Author

unmountComponentAtNode and findDOMNode are mostly there for explicit React API parity.

I'm working on a safari mobile diffing issue

Hydration related?

…viroment doesn't break exports

- add tests for rendering to fragment and handling duplicate keys
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 28e17c4 on V8 into ** on master**.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling d845bcf on V8 into ** on master**.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling b076cab on V8 into ** on master**.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 1ec562e on V8 into ** on master**.

@Zolmeister
Copy link

8.0.0-beta.0 deployed at https://gwent.io and https://deckswap.com
So far so good

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling bbcba6a on V8 into ** on master**.

@thysultan
Copy link
Member Author

Are they both server rendered?

@Zolmeister
Copy link

The last commit did not include new/updated tests?

And yes, though note I'm using dio.js as the backend for Zorium (dio branch) which uses a component instance model that guarantees state persistence across DOM tree manipulation (fundamentally different from React)

For server-side rendering I crawl the vtree to fill state before stringification, and bail if too slow. (untilStable())

@thysultan
Copy link
Member Author

thysultan commented Oct 5, 2017

The last commit did not include new/updated tests?

No, nothing changed that would requires new/updated tests.

I think the last point to settle on before release is whether to include the renderer package in the next minor release or with the 8.0 release.

@Zolmeister
Copy link

Because it opens up such a large public API, it might be best to let 8.0 settle a bit before introducing it

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling d3bbe6c on V8 into ** on master**.

@thysultan
Copy link
Member Author

I agree, lets settle with that.

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.

None yet

3 participants