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 'contextConsumer' #165

Merged
merged 3 commits into from
Jun 3, 2019
Merged

Conversation

0x53A
Copy link
Contributor

@0x53A 0x53A commented Jun 3, 2019

closes #164

Note: this is currently completely untested.

@0x53A
Copy link
Contributor Author

0x53A commented Jun 3, 2019

It currently works on the client side, but not in SSR:

let ctx = createContext("de")

let inline consume() = contextConsumer ctx (fun lang -> str lang)

// ...

let view model =

// ...

                str "This should be the default value (de):"
                consume()
                contextProvider ctx model.Language [
                    str (sprintf "This should be the model value (%s):" model.Language)
                    consume()
                    contextProvider ctx "xyz" [
                        str (sprintf "This should be the overridden value (%s):" "xyz")
                        consume()
                    ]
                    str (sprintf "This should be the model value (%s):" model.Language)
                    consume()
                ]
                str "This should be the default value (de):"
                consume()

Clientside:

image

SSR:

image

I think it renders the elements too early, so that my stack pushing/popping is completely useless ...

Since SSR is currently incorrect, anyway, this is not a regression, so i think I'll revert that part and just PR the new 'contextConsumer' function.

What do you think?

@0x53A 0x53A changed the title WIP: add 'contextConsumer', improve SSR context handling add 'contextConsumer' Jun 3, 2019
@alfonsogarciacaro
Copy link
Member

This is due to my "cheap solution" in order to support the context API in .NET without many code changes. As it's currently difficult to pass a context object through the nested rendered elements in SSR, I'm just using the default value and hoping the proper one will be used on the client side when mounting React. Does it happen like this in your example?

Maybe it'd be better to fix it in SSR (your note in the comment is already helpful!) but I'm not sure what would be the easiest way to do this. Any ideas?

In any case, thanks for the PR!

/// More info at https://reactjs.org/docs/context.html#contextconsumer
let inline contextConsumer (ctx: IContext<'T>) (children: 'T -> ReactElement): ReactElement =
#if FABLE_COMPILER
ReactBindings.React.createElement(ctx?Consumer, createObj [], [!!children])
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 we can pass null here instead of createObj [].

@alfonsogarciacaro alfonsogarciacaro merged commit 78291f4 into fable-compiler:master Jun 3, 2019
@0x53A
Copy link
Contributor Author

0x53A commented Jun 3, 2019

'm just using the default value and hoping the proper one will be used on the client side when mounting React. Does it happen like this in your example?

Yes, after the first client-side render all is ok.

In my use-case (localization) it doesn't matter anyway, there is only one context anyway.

Maybe it'd be better to fix it in SSR (your note in the comment is already helpful!) but I'm not sure what would be the easiest way to do this. Any ideas?

No simple idea, the rendering would need to be delayed so that the parent can influence the child.

@alfonsogarciacaro
Copy link
Member

Ok, if the client side fixes this let's consider it good enough for now ;) Released as 5.2.4, thank you!

@0x53A 0x53A deleted the context branch October 4, 2019 15:39
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.

How should I use the Context api?
2 participants