do not automatically remove files not generated by Relay #3700

zth · 2021-12-21T17:11:43Z

This ensures that the Rust compiler does not automatically remove anything from the artifact folders that wasn't generated by Relay itself. This restores the behavior of the JS compiler, that didn't remove non-Relay files (or at least could be configured so that it didn't).

I'm unsure if this is the best place to put a check like this. Ideally I guess the non-Relay files shouldn't be picked up at all by the compiler. But here I had access to the config I needed, which is why it ended up here.

Comments appreciated!

…t folders

alunyov · 2021-12-28T15:42:49Z

compiler/crates/relay-compiler/src/build_project/mod.rs

@@ -205,6 +206,14 @@ pub fn build_programs(
    Ok((programs, Arc::new(source_hashes)))
 }

+fn is_relay_file_path(language: &TypegenLanguage, path: &PathBuf) -> bool {
+    match (path.to_str(), &language) {


do we need to convert this to_str()?

can you use this: https://doc.rust-lang.org/std/path/struct.Path.html#method.ends_with?

Yes, I think so, because it seems ends_with there only checks for entire chunks of the path. So that ends_with would mean the entire last chunk (the full file name in this case), which isn't what we're after. I tried using that function first. But maybe there's a better alternative still.

alunyov · 2021-12-28T16:27:13Z

compiler/crates/relay-compiler/src/build_project/mod.rs

@@ -373,7 +382,9 @@ pub async fn commit_project(
                    break;
                }
                let path = config.root_dir.join(remaining_artifact);
-                config.artifact_writer.remove(path)?;
+                if is_relay_file_path(&project_config.typegen_config.language, &path) {


This is mostly nits (and not sure if it makes it better). But can we move this function to the artifact_writer?

We already have should_write:

relay/compiler/crates/relay-compiler/src/build_project/artifact_writer.rs

Line 21 in e72ea12

fn should_write(&self, path: &PathBuf, content: &[u8]) -> Result<bool, BuildProjectError>;

I think we can also add should_remove?

should_remove can just return a boolean instead of Result<...>.

I'd need to make sure that the typegen config is added to the artifact_writer if so, but maybe it's ok to add that? Or I could do a typegen unaware check and just check for .graphql.js or .graphql.ts always. What do you think?

I think it's fine to do a check without typegen config.

What do you think about alternative approach, where we would filter these non-relay artifacts in the file_categorizer?

Somewhere here:

relay/compiler/crates/relay-compiler/src/file_source/file_categorizer.rs

Lines 237 to 239 in a07750f

if let Some(project_name) = self.generated_dir_mapping.find(path) {

return Ok(FileGroup::Generated { project_name });

}

This commit upgrades Relay to version 13 and switches from the old Relay Compiler to the new Rust one (since both things go together). Basically, the main change is that now we have only one Relay config for the whole monorepo and the compiler is being executed for the whole monorepo as well (while being much faster). Additionally, Relay support is directly integrated into Flow so in many cases I simply removed previous Flow types (see `useLazyLoadQuery` and `useFragment`). There is still ongoing effort to improve the Flow types in Relay so not everything is finalized. For this reason I decided to use "Compat" types mode. Similarly, some hooks (`useMutation` and `usePreloadedQuery` for example) still require explicit types information so I didn't change these yet. Regardless of that, we are pretty close to use "Final" types. We just need to wait for the Relay team to finish everything. Many issues were already resolved but there are still some that need to be fixed (not blocking this PR): - facebook/relay#3700 - relayjs/eslint-plugin-relay#131 - prettier/prettier#6102 Important links with additional information: - https://relay.dev/blog/2021/12/08/introducing-the-new-relay-compiler/ - https://github.com/facebook/relay/releases/tag/v13.0.0 - https://github.com/facebook/relay/releases/tag/v13.0.0-rc.0 - https://github.com/facebook/relay/releases/tag/v13.0.0-rc.1 - https://github.com/facebook/relay/releases/tag/v13.0.0-rc.2

facebook-github-bot · 2022-01-07T19:28:42Z

@alunyov has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

This commit upgrades Relay to version 13 and switches from the old Relay Compiler to the new Rust one (since both things go together). Basically, the main change is that now we have only one Relay config for the whole monorepo and the compiler is being executed for the whole monorepo as well (while being much faster). Additionally, Relay support is directly integrated into Flow so in many cases I simply removed previous Flow types (see `useLazyLoadQuery` and `useFragment`). There is still ongoing effort to improve the Flow types in Relay so not everything is finalized. For this reason I decided to use "Compat" types mode. Similarly, some hooks (`useMutation` and `usePreloadedQuery` for example) still require explicit types information so I didn't change these yet. Regardless of that, we are pretty close to use "Final" types. We just need to wait for the Relay team to finish everything. Many issues were already resolved but there are still some that need to be fixed (not blocking this PR): - facebook/relay#3700 - relayjs/eslint-plugin-relay#131 - prettier/prettier#6102 Important links with additional information: - https://relay.dev/blog/2021/12/08/introducing-the-new-relay-compiler/ - https://github.com/facebook/relay/releases/tag/v13.0.0 - https://github.com/facebook/relay/releases/tag/v13.0.0-rc.0 - https://github.com/facebook/relay/releases/tag/v13.0.0-rc.1 - https://github.com/facebook/relay/releases/tag/v13.0.0-rc.2

This commit upgrades Relay to version 13 and switches from the old Relay Compiler to the new Rust one (since both things go together). Basically, the main change is that now we have only one Relay config for the whole monorepo and the compiler is being executed for the whole monorepo as well (while being much faster). Additionally, Relay support is directly integrated into Flow so in many cases I simply removed previous Flow types (see `useLazyLoadQuery` and `useFragment`). There is still ongoing effort to improve the Flow types in Relay so not everything is finalized. For this reason I decided to use "Compat" types mode. Similarly, some hooks (`useMutation` and `usePreloadedQuery` for example) still require explicit types information so I didn't change these yet. Regardless of that, we are pretty close to use "Final" types. We just need to wait for the Relay team to finish everything. Many issues were already resolved but there are still some that need to be fixed (not blocking this PR): - facebook/relay#3700 - relayjs/eslint-plugin-relay#131 - prettier/prettier#6102 Important links with additional information: - https://relay.dev/blog/2021/12/08/introducing-the-new-relay-compiler/ - https://github.com/facebook/relay/releases/tag/v13.0.0 - https://github.com/facebook/relay/releases/tag/v13.0.0-rc.0 - https://github.com/facebook/relay/releases/tag/v13.0.0-rc.1 - https://github.com/facebook/relay/releases/tag/v13.0.0-rc.2 adeira-source-id: bc3d10741250e32b6fd399245f62d25484c6ae70

…ctories Summary: This diff is an alternative to this PR: #3700 Relay removes all files in the generated directories. And GIT could not keep empty directories, so people have to add .gitkeep/README or similar to push directories. This should address these issues: - #3727 - #3782 Reviewed By: mofeiZ Differential Revision: D33893146 fbshipit-source-id: 665f9ec07e3c2650e8b1d6e0fed7f9fdd8da201e

alunyov · 2022-02-01T01:57:21Z

Hey @zth I have a slightly different fix for this: 0b5a8b9

We now can ignore files in generated directories if they are not "potentially" created with relay-compiler. Will it work for your use case?

zth · 2022-02-01T07:02:05Z

@alunyov that's looking great! Sorry for dropping the ball on this, I was trying to do something similar to that approach, but got stuck.

I'm closing this in favor for yours, thank you for solving it!

do not automatically remove files not generated by Relay from artifac…

6f1f2db

…t folders

facebook-github-bot added the CLA Signed label Dec 21, 2021

alunyov reviewed Dec 28, 2021

View reviewed changes

alunyov approved these changes Dec 28, 2021

View reviewed changes

mrtnzlml mentioned this pull request Dec 31, 2021

Universe: use Relay 13 and the new Rust Compiler adeira/universe#2119

Merged

alunyov mentioned this pull request Jan 7, 2022

[v13] Compilier does not create artifactDirectory if does not exist #3727

Closed

mrtnzlml mentioned this pull request Jan 9, 2022

Rust compiler: purging non-Relay files from __generated__ folders #3438

Open

zth closed this Feb 1, 2022

zth deleted the do-not-delete-non-relay-files branch February 1, 2022 07:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

do not automatically remove files not generated by Relay #3700

do not automatically remove files not generated by Relay #3700

Uh oh!

zth commented Dec 21, 2021

Uh oh!

alunyov Dec 28, 2021 •

edited

Loading

Uh oh!

zth Dec 28, 2021

Uh oh!

alunyov Dec 28, 2021

Uh oh!

alunyov Dec 28, 2021

Uh oh!

zth Dec 29, 2021

Uh oh!

alunyov Dec 29, 2021

Uh oh!

facebook-github-bot commented Jan 7, 2022

Uh oh!

alunyov commented Feb 1, 2022

Uh oh!

zth commented Feb 1, 2022

Uh oh!

Uh oh!

	if let Some(project_name) = self.generated_dir_mapping.find(path) {
	return Ok(FileGroup::Generated { project_name });
	}

do not automatically remove files not generated by Relay #3700

do not automatically remove files not generated by Relay #3700

Uh oh!

Conversation

zth commented Dec 21, 2021

Uh oh!

alunyov Dec 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zth Dec 28, 2021

Choose a reason for hiding this comment

Uh oh!

alunyov Dec 28, 2021

Choose a reason for hiding this comment

Uh oh!

alunyov Dec 28, 2021

Choose a reason for hiding this comment

Uh oh!

zth Dec 29, 2021

Choose a reason for hiding this comment

Uh oh!

alunyov Dec 29, 2021

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Jan 7, 2022

Uh oh!

alunyov commented Feb 1, 2022

Uh oh!

zth commented Feb 1, 2022

Uh oh!

Uh oh!

alunyov Dec 28, 2021 •

edited

Loading