Relay: @refetchable doesn't generate correct paths

[mrtnzlml]

Hi! 👋 I know @refetchable is still experimental but I'd like to report one issue which seems to be broken for people outside of FB (non-haste?). This is my component:

export default createRefetchContainer(LocationsPaginatedRefetch, {
  data: graphql`
    fragment LocationsPaginatedRefetch_data on RootQuery
      @argumentDefinitions(
        count: { type: "Int", defaultValue: 20 }
        after: { type: "String" }
      )
      @refetchable(queryName: "LocationsPaginatedRefetchRefetchQuery") {
      incrementalPagination: allLocations(first: $count, after: $after)
        @connection(key: "allLocations_incrementalPagination") {
        edges {
          node {
            id
            ...Location_location
          }
        }
        pageInfo {
          endCursor
        }
      }
    }
  `,
});

The generated graphql file contains this:

"refetch": {
  "connection": {
    "forward": {
      "count": "count",
      "cursor": "after"
    },
    "backward": null,
    "path": (v0/*: any*/)
  },
  "operation": require('LocationsPaginatedRefetchRefetchQuery.graphql'),   // <<<
  "fragmentPathInResult": []
}

I would expect this instead:

"operation": require('./LocationsPaginatedRefetchRefetchQuery.graphql'),

Thanks for checking it. :)

[alunyov]

Yeah, this probably should be replaced by something similar to this:
https://github.com/facebook/relay/blob/master/packages/babel-plugin-relay/createModernNode.js#L54

[BruceL33t]

Having the same problem.

[babangsund]

Funny - I concluded the issue was actually located outside of the babel compilation. 🤔

This could be fixed with a small change here. https://github.com/facebook/relay/blob/cf5566eb0fbee1f75f821d27ad738c23788cea6c/packages/relay-compiler/codegen/writeRelayGeneratedFile.js#L27

Instead, you could just add the relative path and config extension.

function printRequireModuleDependency(extension: string): string => string {
  return moduleName => `require('./${moduleName + extension}')`;
}

And passing the extension...

https://github.com/facebook/relay/blob/cf5566eb0fbee1f75f821d27ad738c23788cea6c/packages/relay-compiler/codegen/writeRelayGeneratedFile.js#L57

... Like so:

  printModuleDependency: (
    moduleName: string,
  ) => string = printRequireModuleDependency(extension),

This could of course have other implications. 😅

EDIT:
Closed duplicate issue

I was playing around with the relay-experimental package when I stumbled upon an issue.
I'm fully aware it isn't ready yet, however this did seem like an issue to me. 😅

Assuming the query:

fragment ViewerFragment on Viewer @refetchable(queryName: "ViewerFragmentRefetchQuery") {
   id
   name
}

The __generated__/ViewerFragmentRefetchQuery.graphql.js file is then generated.
(Assuming the WriterConfig.extension hasn't been altered)

By using the CodeMarker.moduleDependency method, >https://github.com/facebook/relay/blob/cf5566eb0fbee1f75f821d27ad738c23788cea6c/packages/relay-compiler/codegen/ReaderCodeGenerator.js#L76 the moduleName is set to >ViewerFragmentRefetchQuery.graphql, seemingly to be extended with the configured file extension later on. (i.e. .js)

In the printRequireModuleDependency function however, >https://github.com/facebook/relay/blob/c203d54e75d71e0ca1c2c3c8385f7814a5729dfe/packages/relay-compiler/codegen/writeRelayGeneratedFile.js#L28 the moduleName is only ever >converted back to the original .graphql extended moduleName, neglecting the file extension.

This outputs require('ViewerFragmentRefetchQuery.graphql') rather than require('./ViewerFragmentRefetchQuery.graphql.js') which points to the generated refetch query >file.

I'm thinking this is not intentional? 🤔

[babangsund]

@mrtnzlml My suggested solution above was just merged, so this issue could probably be closed. 🙂

[alloy]

@babangsund @mrtnzlml I’ve been reading through the change and these issues and am not clear on why there was a need to add an explicit extension, can you elaborate? This change is making it harder to compile all TS files to JS with a separate tsc step, as the require statement would still explicitly be referring to a .ts file.

[mrtnzlml]

@alloy I am not sure about the extension either. The original issue I reported was about the haste import only:

- "operation": require('LocationsPaginatedRefetchRefetchQuery.graphql'),
+ "operation": require('./LocationsPaginatedRefetchRefetchQuery.graphql'),

I think it got mixed up with this one? https://github.com/facebook/relay/issues/2844

[alloy]

@mrtnzlml Probably yeah, I’ll await @babangsund then 👍

[alloy]

Spoke with @babangsund offline.

babangsund: I think it was because prior to the change, relay was generating these files without any file extension at all
alloy: I see. Yeah obviously the files need the right extension, but the require statement probably doesn’t need it then
babangsund: Probably not

So it seems like we could revert the extension change.