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

Bug: Flow error when using --eagerESModules and @refetchable #3404

Open
mrtnzlml opened this issue Mar 12, 2021 · 2 comments
Open

Bug: Flow error when using --eagerESModules and @refetchable #3404

mrtnzlml opened this issue Mar 12, 2021 · 2 comments

Comments

@mrtnzlml
Copy link
Contributor

Hola! 👋 This issue is for Relay 11.0.0. I created a reproducible demo here: https://github.com/mrtnzlml/bug-relay-useRefetchableFragment-flow

Specifically, the issue is with this line: https://github.com/mrtnzlml/bug-relay-useRefetchableFragment-flow/blob/53ee2bf03395037e72ad306f99df801ff4639491/__generated__/bugRelayUseRefetchableFragmentFlow.graphql.js#L36

Problem

Flow throws an error when generating artifacts with --eagerESModules and @refetchable:

Error ┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈ __generated__/bugRelayUseRefetchableFragmentFlow.graphql.js:36:20

Cannot assign object literal to node because property bugRelayUseRefetchableFragmentFlowQuery is missing in
ConcreteRequest [1] but exists in module ./bugRelayUseRefetchableFragmentFlowQuery.graphql.js [2] in property
metadata.refetch.operation. [incompatible-type]

     __generated__/bugRelayUseRefetchableFragmentFlow.graphql.js
     33│     "refetch": {
     34│       "connection": null,
     35│       "fragmentPathInResult": [],
 [2] 36│       "operation": require('./bugRelayUseRefetchableFragmentFlowQuery.graphql.js')
     37│     }
     38│   },
     39│   "name": "bugRelayUseRefetchableFragmentFlow",

     node_modules/relay-runtime/util/ReaderNode.js.flow
 [1] 68│   +operation: string | ConcreteRequest,



Found 1 error

Steps to reproduce

  1. clone https://github.com/mrtnzlml/bug-relay-useRefetchableFragment-flow
  2. yarn install
  3. yarn run relay-compiler --src=. --schema=schema.graphql --eagerESModules
  4. yarn run flow 💥

Possible solution

The artifact could be generated with require(…).default when using --eagerESModules option like so:

  "metadata": {
    "refetch": {
      "connection": null,
      "fragmentPathInResult": [],
-     "operation": require('./bugRelayUseRefetchableFragmentFlowQuery.graphql.js')
+     "operation": require('./bugRelayUseRefetchableFragmentFlowQuery.graphql.js').default
    }
  }

(no Flow errors)

@mrtnzlml mrtnzlml changed the title Flow error when using --eagerESModules and @refetchable Bug: Flow error when using --eagerESModules and @refetchable Mar 12, 2021
@sibelius sibelius added the bug label Mar 12, 2021
mrtnzlml added a commit to adeira/universe that referenced this issue Mar 12, 2021
This issue affects upstream as well, see: facebook/relay#3404

We have the advantage of having the generated output under control so I fixed it here for `@adeira/relay` and reported it to the upstream.
kodiakhq bot pushed a commit to adeira/universe that referenced this issue Mar 12, 2021
This issue affects upstream as well, see: facebook/relay#3404

We have the advantage of having the generated output under control so I fixed it here for `@adeira/relay` and reported it to the upstream.
adeira-github-bot pushed a commit to adeira/relay that referenced this issue Mar 12, 2021
This issue affects upstream as well, see: facebook/relay#3404

We have the advantage of having the generated output under control so I fixed it here for `@adeira/relay` and reported it to the upstream.

adeira-source-id: 079091984297d98698575e38e48da94aff601017
@TrySound
Copy link
Contributor

Using "require" with eagerESModules is not an option in any case. You can convert to esm like this.

const javascript = require('relay-compiler/lib/language/javascript/RelayLanguagePluginJavaScript.js');
const { formatGeneratedModule } = require('relay-compiler');

module.exports = {
  ...
  language: () => ({
    ...javascript(),
    formatModule: options => {
      // replace commonjs export
      let formattedModule = formatGeneratedModule(options);
      formattedModule = formattedModule.replace(
        'module.exports = ',
        'export default ',
      );

      // replace requires
      let i = -1;
      const paths = [];
      formattedModule = formattedModule.replace(
        /require\(('.+')\)/g,
        (req, pathString) => {
          i += 1;
          paths.push(pathString);
          return `__import__${i}`;
        },
      );

      // add new imports
      const imports = paths.map((p, i) => `import __import__${i} from ${p};\n`);
      formattedModule = formattedModule.replace(
        /'use strict';\n/,
        `'use strict';\n\n${imports.join('')}`,
      );

      return formattedModule;
    },
  }),
};

@stale
Copy link

stale bot commented Jan 9, 2022

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.

@stale stale bot added the wontfix label Jan 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants