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

[Flight Fixture] Refetching #20316

Closed
wants to merge 4 commits into from
Closed

[Flight Fixture] Refetching #20316

wants to merge 4 commits into from

Conversation

gaearon
Copy link
Collaborator

@gaearon gaearon commented Nov 22, 2020

Adds basic Flight payload refetching to the fixture. Still no client cache.

@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Nov 22, 2020
@codesandbox-ci
Copy link

codesandbox-ci bot commented Nov 22, 2020

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 208360e:

Sandbox Source
React Configuration

@gaearon gaearon force-pushed the refetch branch 2 times, most recently from 165f57d to 314d0ae Compare November 22, 2020 23:33
@sizebot
Copy link

sizebot commented Nov 22, 2020

No significant bundle size changes to report.

Size changes (stable)

Generated by 🚫 dangerJS against 208360e

@sizebot
Copy link

sizebot commented Nov 22, 2020

No significant bundle size changes to report.

Size changes (experimental)

Generated by 🚫 dangerJS against 208360e


function refresh() {
setData(
ReactTransportDOMClient.createFromFetch(fetch('http://localhost:3001'))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Once we have it, this should use the client cache and it's the cache that gets reset.

However, even in that world this can be optimized by seeding the client cache with a new entry.

The idiomatic way would be to seed it with a response that comes from the same HTTP request as the mutation though.

That last part you can prepare for now even without a client cache. You just need to pass this in to the refresh function as an argument.

Copy link
Collaborator

@sebmarkbage sebmarkbage Dec 7, 2020

Choose a reason for hiding this comment

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

Just to be explicit. The reason for this is not perf but eventual consistency. You can easily get into a state where you never see your mutation in prod because the propagation is always slower than immediately handling the next request. That's a really bad user experience and bad developer experience to only know about it once you hit prod.

}

ReactDOM.render(
Copy link
Collaborator

Choose a reason for hiding this comment

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

omg

'Content-Type': 'application/json',
},
body: JSON.stringify({text}),
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

You can have this fetch return a new Flight response. It means you'll have to hard code which Flight root entry point this corresponds to. That's kind of a hard problem for a router but it's good to indicate that it is a problem that's worth solving.

@gaearon gaearon closed this Feb 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants