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

Can't resolve graphl file when using Webpack 5 (and nextjs) #3272

Closed outerlook closed 10 months ago

outerlook commented 3 years ago

Hi.

Using:

But when I upgrade to webpack 5, which is supported by nextjs I get the following error while starting development server or building:

Module not found: Can't resolve '..\..\__generated__\relay\someMutation.graphql' this is the graphql file added by babel relay plugin.

I've been trying to debug and understand better babel's work but I confess I do not have much knowledge to debug it further by myself.

babel.config.json

{
  "presets": [
    "next/babel"
  ],
  "plugins": [
    [
      "relay",
      {
        "artifactDirectory": "./src/__generated__/relay/"
      }
    ],
    "@babel/plugin-transform-runtime",
    "babel-plugin-macros",
    [
      "styled-components",
      {
        "ssr": true,
        "displayName": true
      }
    ]
  ]
}

Note that I use babel plugin macros for tailwind's macro, not relay plugin macro. Although I tried using it and I got the same error too...

My generated files are indeed correctly generated at '.src/__generated__/relay'

It does work with webpack 4, and not with webpack 5.

I appreciate any help to try debugging it, I didn't find any public repo that uses typescript, relay and webpack 5, that's why I'm not sure I'm the one doing something wrong.

outerlook commented 3 years ago

strangely, a workaround is to remove artifactDirectory option from both react-compiler options and babel-relay-plugin... then it generates a __generated__ folder on each file that uses a graphql tag. It messes up my codebase but it works until we figure out how to get it to work like before on webpack 5

CoericK commented 3 years ago

Hey @outerlook, did you manage to solve? I do have the same stack, and noticed that it only happens on Windows environments on Unix it works just fine. I will try the work around you suggested

vgedzius commented 3 years ago

The real culprit seems to be here: https://github.com/relay-tools/relay-compiler-language-typescript#typescript

Relay compiler requires module to be set to "es2015" in compilerOptions of tsconfig, however next automatically, sets it to "esnext". I am yet to find a workaround.

farshid1 commented 3 years ago

@vgedzius any luck??

CrocoDillon commented 3 years ago

I run into the same issue:

Module not found: Can't resolve '..\..\.relay\someQuery.graphql'
Did you mean './..\..\.relay\someQuery.graphql'?
Requests that should resolve in the current directory need to start with './'.

Not sure where that error is coming from but at least I found the reason why the path doesn't start with './':

https://github.com/facebook/relay/blob/521ca914ec756d8c8d6f291dde93c3eccf1ae56f/packages/babel-plugin-relay/compileGraphQLTag.js#L236-L237

Apparently that was fine in Webpack 4 but not in Webpack 5? If the error comes from Webpack that is. By the way forcing a './' here did work around the issue. Shouldn't be needed for paths already starting with '../' if you ask me.

At least you can tell Webpack 5 to prefer relative; in the case of NextJS:

// next.config.js
module.exports = {
  webpack: config => {
    config.resolve.preferRelative = true
    return config
  },
}
artola commented 3 years ago

I confirm that for us also in Windows we see this error (except with "module": "commonjs" where it works).

It seems related to the joinPath (alias for path.join) because it is environment sensitive.

If we "patch" the code (by replacing the slashes) in the following line to make it POSIX ... then it works everywhere, independently of the settings in tsconfig.

https://github.com/facebook/relay/blob/ac930cd215152057114933c20d5fe1b02e2bc3bb/packages/babel-plugin-relay/compileGraphQLTag.js#L239

return relativeReference + joinPath(relative, fileToRequire).replace(/\\/g, '/');

@alloy @kassens Do you have some idea to define if it a problem of this plugin or next in the chain? (for example some change in webpack API between v4 and v5 regarding normalisation of the paths)

See: Tobias' comment https://blog.johnnyreilly.com/2021/04/20/ts-loader-goes-webpack-5/

Note that asset names should have only / as they are filenames and not paths. Only absolute paths have \.

But what about test environment ? Then, might be better if webpack normalise the filenames as somehow was happening in v4. It would be great to have their team participating in this issue.

sokra commented 3 years ago

getRelativeImportPath is incorrect for windows. Requests in imports should always use / but path.relative returns a path which contains \.

artola commented 3 years ago

@sokra Do you think that this should be fixed in the relay plugin? Why before in v4 this code that it is 2 years old was passing?

artola commented 3 years ago

@jstejada @josephsavona I am of the same opinion than @sokra above and I caught in this because our Windows users started to complain while other users on mac or server in linux has not detected any problem. As a fast solution, I wrote a small Babel plugin to fix the require only for process.platform === 'win32' running just after babel-plugin-relay and since then no problems anymore, so happy webpack and jest in any environment.

Advise: if you want to use path.relative but normalise it with .replace(/\\/g, '/'); . Please let me know if you prefer a PR.

artola commented 3 years ago

If someone experience this problem, might be interested to use this plugin that I did, just add it to your babel.config.js after the relay plugin. It is in the meantime, until this problem is fixed and a new babel-plugin-relay is published.

...
plugins: [
  ['babel-plugin-relay', { ... }],
  ['./pathNormaliser'],
  ...
],
...
'use strict';

const replacer = (filepath) => filepath.replace(/\\/g, '/');

/**
 * Normalise path to POSIX (only for `win32` using `require`).
 */
const PathNormaliserPlugin = function (api) {
  if (process.platform === 'win32') {
    const t = api.types;

    return {
      name: 'path-normaliser',

      visitor: {
        CallExpression: {
          enter: function (nodePath) {
            const callee = nodePath.get('callee');

            if (callee.isIdentifier() && callee.equals('name', 'require')) {
              const arg = nodePath.get('arguments.0');

              if (arg && arg.isStringLiteral()) {
                const sourcePath = arg.node.value;
                const targetPath = replacer(sourcePath);

                if (sourcePath !== targetPath) {
                  arg.replaceWith(t.stringLiteral(targetPath));
                }
              }
            }
          },
        },
      },
    };
  }

  return {
    name: 'path-normaliser',
  };
};

module.exports = PathNormaliserPlugin;
artola commented 3 years ago

@alunyov Could you please advice how to proceed? is FB team fix ongoing? or should we PR?

JUNKYASS commented 3 years ago

I confirm that for us also in Windows we see this error (except with "module": "commonjs" where it works).

It seems related to the joinPath (alias for path.join) because it is environment sensitive.

If we "patch" the code (by replacing the slashes) in the following line to make it POSIX ... then it works everywhere, independently of the settings in tsconfig.

https://github.com/facebook/relay/blob/ac930cd215152057114933c20d5fe1b02e2bc3bb/packages/babel-plugin-relay/compileGraphQLTag.js#L239

return relativeReference + joinPath(relative, fileToRequire).replace(/\\/g, '/');

@alloy @kassens Do you have some idea to define if it a problem of this plugin or next in the chain? (for example some change in webpack API between v4 and v5 regarding normalisation of the paths)

See: Tobias' comment https://blog.johnnyreilly.com/2021/04/20/ts-loader-goes-webpack-5/

Note that asset names should have only / as they are filenames and not paths. Only absolute paths have \.

But what about test environment ? Then, might be better if webpack normalise the filenames as somehow was happening in v4. It would be great to have their team participating in this issue.

Thank you! I found package "babel-relay-plugin" and changed compileGraphQLTag.js file as you did. Now it works!

giautm commented 3 years ago

@JUNKYASS you shouldn't change directly file compileGraphQLTag.js. It will reset after reinstall. You should follow this comment instead

https://github.com/facebook/relay/issues/3272#issuecomment-912950844

JUNKYASS commented 3 years ago

@JUNKYASS you shouldn't change directly file compileGraphQLTag.js. It will reset after reinstall. You should follow this comment instead

#3272 (comment)

Yes, you're right. Got it, thanks!

stale[bot] commented 2 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.

artola commented 2 years ago

@alunyov This happens even with v13 since the code has not changed in the involved lines mentioned above:

https://github.com/facebook/relay/blame/b5add1e0369ee38150d810c5c677c57d0952670e/packages/babel-plugin-relay/compileGraphQLTag.js

I do appreciate the advise of @sokra that confirms the bug:

https://github.com/facebook/relay/issues/3272#issuecomment-912781359

This is just affecting non posix (Windows) and the required fix it is just a few characters long, should I create a PR for this or are you thinking to apply a different solution?

For example, in place of a regex replacement, a more resilient solution will be to use = require('path').posix; and all the methods will use the right separator. This is better because even if it might be strange to use the character \ within the file/folder names, in posix it is so valid like "a".

https://github.com/facebook/relay/blob/35e0c2bd4fd020a1bd5ac467e50adb5ad5f57d40/packages/babel-plugin-relay/compileGraphQLTag.js#L27

wKich commented 2 years ago

@artola I like that way. What's a problem with using this solution?

artola commented 2 years ago

Fix getting relative import path in windows for babel-plugin-relay #3830

@wKich Thanks for the PR. From my point of view, this is the simplest solution and also Windows' compatible.

Phat-Loc commented 2 years ago

Does any have a repo working that uses Relay 13, Webpack 5 and typescript? There is a bunch of module resolution errors that I am unable resolve. Thanks!

giautm commented 2 years ago

Hey, if you're stuck with this issue, please try to use patch-package to apply the fix.

diff --git a/node_modules/babel-plugin-relay/lib/compileGraphQLTag.js b/node_modules/babel-plugin-relay/lib/compileGraphQLTag.js
index 39a21e6..928c8de 100644
--- a/node_modules/babel-plugin-relay/lib/compileGraphQLTag.js
+++ b/node_modules/babel-plugin-relay/lib/compileGraphQLTag.js
@@ -14,7 +14,7 @@ var crypto = require('crypto');
 var _require = require('graphql'),
     print = _require.print;

-var _require2 = require('path'),
+var _require2 = require('path/posix'),
     dirname = _require2.dirname,
     joinPath = _require2.join,
     relativePath = _require2.relative,
wKich commented 2 years ago

@artola I checked usage of require('path').posix and it doesn't fix backslashes in my case. So I think replace(/\\/g, '/') is the best solution, but we need to apply it only for windows

artola commented 2 years ago

@artola I checked usage of require('path').posix and it doesn't fix backslashes in my case. So I think replace(/\\/g, '/') is the best solution, but we need to apply it only for windows

@giautm could you please confirm if your path working on Windows as expected?

sibelius commented 11 months ago

is this fixed?

artola commented 11 months ago

is this fixed?

Yesterday, I checked the babel-plugin-relay code in the mentioned lines, and I can confirm that there have been no changes to this code for several years.

image

As explained by @sokra here, the need for these changes still persists.

artola commented 11 months ago

If someone needs it, here's a plugin that replaces the slashes, now updated to support eagerEsModules. It serves as a workaround; however, we should submit a proper fix as explained above.

'use strict';

/**
 * Normalise path to POSIX (only for `win32`).
 */
const PathNormaliserPlugin = function (api) {
  if (process.platform === 'win32') {
    const t = api.types;

    const normalizePath = (pathNode) => {
      if (pathNode && pathNode.isStringLiteral()) {
        const sourcePath = pathNode.node.value;
        const targetPath = sourcePath.replace(/\\/g, '/');

        if (sourcePath !== targetPath) {
          pathNode.replaceWith(t.stringLiteral(targetPath));
        }
      }
    };

    return {
      name: 'path-normaliser',

      visitor: {
        CallExpression: {
          enter(nodePath) {
            const callee = nodePath.get('callee');

            if (callee.isIdentifier({name: 'require'})) {
              normalizePath(nodePath.get('arguments.0'));
            }
          },
        },
        ImportDeclaration: {
          enter(nodePath) {
            normalizePath(nodePath.get('source'));
          },
        },
      },
    };
  }

  return {
    name: 'path-normaliser',
  };
};

module.exports = PathNormaliserPlugin;
artola commented 11 months ago

The above plugin is a workaround; a proper fix is on the way with PR #4544.