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

Add support for cascading titles #37

Merged
merged 3 commits into from
Jul 11, 2018
Merged

Add support for cascading titles #37

merged 3 commits into from
Jul 11, 2018

Conversation

TrySound
Copy link
Collaborator

Ref #11

This PR is cascading titles MVP. My project requires similar to helment
behaviour for titles.

@TrySound
Copy link
Collaborator Author

@tizmagik Could you take a look?

@tizmagik
Copy link
Owner

Awesome to see this! Nice job. I will take a closer look tomorrow; I want to ensure that this implementation is completely thread-safe.

I think it might be possible to test for this in the test suite, actually. What do you think about setting up an async test with two render requests simultaneously and ensure that the responses are properly thread-safe?

@TrySound
Copy link
Collaborator Author

The idea of this implementation is keeping state inside provider so HeadTag could rely on it and render null when a title is not the last one. So this shouldn't work correctly with two react instances.

@TrySound
Copy link
Collaborator Author

Or do you mean parallel subtrees?

@tizmagik
Copy link
Owner

Yes I mean in parallel

@TrySound
Copy link
Collaborator Author

Will add test tomorrow then

@TrySound
Copy link
Collaborator Author

@tizmagik Done. Fixed dynamic insertion. Right now it works only for titles. I need this asap. Can we release 3.0.0-rc.0 under next tag?

Will add support for meta[name] in the next PR. For which tags this behaviour is also matter?

Ref #11

This PR is cascading titles MVP. My project requires similar to helment
behaviour for titles.
@tizmagik
Copy link
Owner

Sure, will publish this branch against @next tag

@TrySound
Copy link
Collaborator Author

Cool. Thanks

@TrySound TrySound merged commit ead3722 into master Jul 11, 2018
@TrySound TrySound deleted the cascading-titles branch July 11, 2018 22:34
@tizmagik tizmagik mentioned this pull request Jul 16, 2018
zhangciwu pushed a commit to zhangciwu/react-head that referenced this pull request Aug 10, 2018
Ref tizmagik#11

This PR is cascading titles MVP. My project requires similar to helment
behaviour for titles.
@tizmagik tizmagik mentioned this pull request Aug 13, 2018
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

2 participants