search

facebook / relay

Relay is a JavaScript framework for building data-driven React applications.
https://relay.dev
MIT License
18.41k stars 1.83k forks source link

[relay-compiler] --persist-output in 2.0.0 and --watch #2625

Open JonathanUsername opened 5 years ago

JonathanUsername commented 5 years ago

It seems --persist-output overwrites the file with only the queries that were changed when using --watch. The expected behaviour would be for it to merge the object or at least to output a queryMap.json with all valid queries. This means that if there are no changes, it's overwritten with an empty object.

To clarify further, simply running it twice renders the output useless:

$ relay-compiler --schema ./schema.graphql --src ./ --persist-output queryMap.json && cat queryMap.json | wc -l
88

$ relay-compiler --schema ./schema.graphql --src ./ --persist-output queryMap.json && cat queryMap.json | wc -l
1

Having spoken with @alloy it seems the issue is in the change from how persisted queries were implemented in https://github.com/facebook/relay/pull/2354 and how they went into 2.0.0 in https://github.com/facebook/relay/commit/708f842c30e3cbb709a6b516002529b7de11dfe3

Specifically, by writing out a new map each time, it clobbers the previous output and has no intermediate state.

This feels like an architectural decision so I'm reluctant to propose a solution, but it seems that managing the state so that you can efficiently output a queryMap of all persisted queries is quite important.

If we don't go back to having the many co-located intermediate .queryMap.json files, you would at least need a way to force re-evaluation of all queries in --watch mode, so that the full new map is emitted each time.

alloy commented 5 years ago

Just for some context on the difference in initial implementation and the one that landed; some state maintenance will be needed to keep track of artifacts for files that were removed. I.e. we can't simply merge the existing query map with new changes. This was previously achieved by deriving the full query map from all the individual artifacts and having the compiler remove artifacts for removed files.

josephsavona commented 5 years ago

Yeah, this is a bug in watch mode due to my changes when importing. We’re happy to accept a fix, but note that the fix should simply keep in-memory state about which files to create/update/delete rather than creating files that it has to clean up.

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.