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

Added preloadWeak static method #120

Merged
merged 3 commits into from
Jun 4, 2018

Conversation

edongashi
Copy link
Contributor

@edongashi edongashi commented May 19, 2018

Added preloadWeak which attempts to resolve and hoist the wrapped component synchronously. This is useful when reading or writing data from/to static properties before initial render.

I'm not sure if this implementation is correct, but I think with the description given in #119 you can understand what the desired behavior is.
Thanks.

PS The name or method of invoking this doesn't matter, you can add an optional parameter to preload or add a preload.sync or whatever.

edongashi and others added 2 commits May 19, 2018 13:17
Added preloadWeak which attempts to resolve and hoist the wrapped component synchronously. This is useful when reading or writing data from/to static properties before initial render.
@ScriptedAlchemy
Copy link
Collaborator

Code looks good to me but I’d appreciate a updated test for this scenario if possible

@ScriptedAlchemy
Copy link
Collaborator

Can you also add an example to the readme? It would be good to write this isecase down

@edongashi
Copy link
Contributor Author

HI, I added some clarification in this section.

Is this sufficient? Or should I create a separate section for the method?

About tests: I need to do some research because I am unfamiliar with react/jest testing. I will update as soon as I get the chance to figure it out. Thanks

Copy link
Collaborator

@ScriptedAlchemy ScriptedAlchemy left a comment

Choose a reason for hiding this comment

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

Yea that’s perfect!

If a test can be created - I’m not sure what we have in there.

I’m approving this in the mean time.

@edongashi
Copy link
Contributor Author

This is basically preload() but without the async fetching. I copied the preload code and removed parts that initiate async requests. I'll probably check the existing tests on preload() and adapt them for this simpler use case.

The desired behavior was to be able to read static properties before rendering. The scenario that inspired this:

SERVER:

const UniversalComponent = universal(... something)

// sometime later

const initialProps = await UniversalComponent.getInitialProps()
renderToDomString(
  <UniversalComponent {...initialProps} />
)

This woulnd't work because UniversalComponent has never rendered, hence static properties were not hoisted. In order to hoist staitcs I had to render it before (same with client). Which made it impossible to access statics before rendering. onAfter is called mid render-cycle which was not ideal for state management.

@ScriptedAlchemy ScriptedAlchemy merged commit bf7f1bf into faceyspacey:master Jun 4, 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.

2 participants