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

fix(useOutlet): don't wrap in ContextProvider if no child route #8483

Merged
merged 3 commits into from Dec 14, 2021

Conversation

mcansh
Copy link
Collaborator

@mcansh mcansh commented Dec 14, 2021

closes #8477

Signed-off-by: Logan McAnsh <logan@mcan.sh>
Signed-off-by: Logan McAnsh <logan@mcan.sh>
@mcansh mcansh requested a review from mjackson December 14, 2021 16:04
@mcansh mcansh changed the title fix(useOutlet): dont wrap in context if no context fix(useOutlet): dont wrap in ContextProvider if no context Dec 14, 2021
Signed-off-by: Logan McAnsh <logan@mcan.sh>
@goffioul
Copy link

I'm confused by the proposed fix. Using useOutlet(null) (or <Outlet context={null} />) is still a valid use case and it doesn't mean that you don't want to provide any context to the child component, but you want to provide a null value for the context.

I could be wrong, but is there a valid use case where you'd still want to render OutletContext.Provider while outlet is null? It seems to me that if outlet is null, useOutlet() should just return null, whether or not a context is provided as argument.

Whether or not to wrap outlet with the context provider is IMO another story. I don't think you can decide solely on the context value, can you?

@mjackson
Copy link
Member

It seems to me that if outlet is null, useOutlet() should just return null, whether or not a context is provided as argument.

@goffioul You are correct. This was fixed in 7d16c39

@mjackson mjackson mentioned this pull request Dec 14, 2021
@mcansh mcansh changed the title fix(useOutlet): dont wrap in ContextProvider if no context fix(useOutlet): dont wrap in ContextProvider if no child route Dec 14, 2021
@mcansh mcansh changed the title fix(useOutlet): dont wrap in ContextProvider if no child route fix(useOutlet): don't wrap in ContextProvider if no child route Dec 14, 2021
@mcansh mcansh merged commit 2b4aed6 into main Dec 14, 2021
@mcansh mcansh deleted the logan/8477 branch December 14, 2021 17:12
mjackson added a commit that referenced this pull request Dec 14, 2021
@mcansh mcansh restored the logan/8477 branch December 14, 2021 17:57
@mcansh mcansh deleted the logan/8477 branch December 14, 2021 20:32
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.

[Bug]: useOutlet() does not return null anymore
3 participants