-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
@preloadable support #4110
@preloadable support #4110
Conversation
Wires the pre-existing `compact` printing into a feature flag. Additionally disables indenting when printing in compact form
Sorry for dropping the ball on this PR.
I think this would be the most feasible path forward, and compatible with what we have internally. This is most likley would be much easier to land, because it won't interfere with the existing codegen we have internally, where the @preloadable artifacts created as part of
I think it is fine to move it to
It is internal optimization that tightly coupled with the way the query persisting work on WWW. I don't think I can think of a good abstraction for that in OSS, we probably may skip this. We also don't use that in React Native, for example.
I think this is just happened - the duplication of these parameters, and now, to change that we need to update many-many artifacts - a complex rollout without a clear benefit.
Overall, the summary and the PR implementation is correct. TL;DR - @preloadable can generate a file with minimal information to fetch the query (query text or query id) - this artifact can be used with preloading/router infra (in entry points) to start fetching the query earlier without loading the full query AST for the normalization. |
Awesome - thanks for all that info. I'll refresh this implementation onto master and iterate from there |
@tomgasson @alunyov I'm currently playing with EntryPoints and really wanted to test out I'm also going to move the @tomgasson If you want to to be attributed further, just let me know. I can add you as Co-Author on my commits :) |
@tobias-tengler Nice! Happy for you to take this on. I don't need attribution, just after the outcomes 😄 |
Summary: This is based on an earlier implementation done by tomgasson in #4110 Companion PR for `types/relay-runtime`: DefinitelyTyped/DefinitelyTyped#68144 An example application using this can be found [here](https://github.com/tobias-tengler/nextjs-relay-entrypoints) Pull Request resolved: #4515 Reviewed By: tyao1 Differential Revision: D52608572 Pulled By: alunyov fbshipit-source-id: d75f904d0f20a89d3542fabf789c6c9d3c8595ce
should we close this? related work #4515 |
Placeholder PR to check plans, before I proceed with the implementation.
My understanding of
@preloadable
:@preloadable
directive on an Query<Query>$Parameters.js
file, which exports{kind: "PreloadableConcreteRequest", params: PARAMS}
where PARAMS is the exact same as the params in the operation, of typePreloadableConcreteRequest<Operation$variables, Operation$data, Operation$rawResponse>
usePreloadedQuery
)PreloadableQueryRegistry.set(node.params.id, node)
. This is used to unblock the usage of the query when the full metadata is loaded.Questions:
generate_extra_artifacts
at Meta currentlygenerate_extra_artifacts_fn
is called incommit_project
, asArtifactContent::Generic
itemsbuild_project
->generate_artifacts
ArtifactContent::PreloadableOperation
and re-uses the artifactArtifactContent::as_bytes
mechanism and much of the other code in that spacePreloadableConcreteRequest
remain in thereact-relay
package? The compiler doesn't have other references toreact-relay
. I can introduce a mechanism inArtifactGeneratedTypes
to collect and organise imports & sources together or movePreloadableConcreteRequest
intorelay-runtime
<Operation>_facebookRelayOperation
). What's this for? Is this needed in OSS?params
from the PCR (as aJSModuleDependency
) rather than duplicating them?I'd appreciate any pointers you can share from Meta's impl of
generate_extra_artifacts
/generate_virtual_id_file_name
cc @voideanvalue