search

martpie / next-transpile-modules

Next.js plugin to transpile code from node_modules. Please see: https://github.com/martpie/next-transpile-modules/issues/291
MIT License
1.13k stars 85 forks source link

Yarn PnP Virtual Workspaces Filepaths Not Properly Transpiled #199

Closed jpulec closed 1 year ago

jpulec commented 3 years ago

Are you trying to transpile a local package or an npm package? I'm trying to transpile sibling workspace packages.

Describe the bug When using Yarn 2 PnP in a monorepo with yarn workspaces, sometimes a workspace may get resolved to a virtual package. Yarn uses virtual packages mainly to handle peerDependencies properly.

The result of this is that calling require.resolve('@myworkspace/common') may return a virtual filesystem path, (i.e. .yarn/$$virtual/@myworkspace-common) instead of the actual on disk location of the package.

This becomes a problem because the generated matcher for next-transpile-modules checks the resolved paths against the real file paths that webpack passes in the include condition of the rule that next-transpile-modules adds to the webpack config.

The result is that the file path does not match and as a result is not transpiled.

To Reproduce

  1. Create a yarn workspace with multiple packages.
  2. Have package-a reference package-b using the workspace: protocol.
  3. Add a peerDependency (like react to package-b)
  4. The resolved paths for package-b will now be virtual and not properly transpiled.

Expected behavior The virtual filepaths should behave the same way as real filepaths, and should be transpiled.

Setup

If I can find a little time, I'll try to put together a full reproduction repo.

martpie commented 3 years ago

Do you have any idea how it could be solved?

davidcalhoun commented 3 years ago

Aug 8, 2022 update: things seem to be working fine now with next@12.2.0 and next-transpile-modules@9.0.0! Keeping the below info for posterity.

Interestingly, I'm experiencing a possibly similar issue after making some minor/patch dependency updates (Next.js 10.1.3 -> 10.2.0), maybe something in Next.js itself changed? Running next-transpile-modules@6.4.0 worked fine with the older version of Next. Note that I'm using Webpack 5, and that xxxxxxxxxxxxxxxxxxx is the name of a yarn workspace.

When I try to build:

yarn pnpify next build

I get this error:

➤ YN0000: Failed to compile.
➤ YN0000: 
➤ YN0000: ../../.yarn/$$virtual/@xxxxxxxxxxxxxxxxxxx-virtual-824b3cd196/1/packages/xxxxxxxxxxxxxxxxxxx/src/components/Avatar/index.tsx
➤ YN0000: Module parse failed: Unexpected token (7:7)
➤ YN0000: You may need an appropriate loader to handle this file type, currently no loaders are configured to process this file. See https://webpack.js.org/concepts#loaders
➤ YN0000: | import { classList } from '../../utils';
➤ YN0000: | 
➤ YN0000: > export type Props = {

Interesting, it's not failing in dev mode. Things work fine with this (because it's not using pnpify?):

yarn next dev

Note that for me it's failing at exporting a type - maybe my issue is different? Seems like some TypeScript webpack loader isn't getting configured correctly? Will try some workaround and update this if I discover anything.

Versions

borekb commented 3 years ago

To Reproduce

  1. Create a yarn workspace with multiple packages.
  2. Have package-a reference package-b using the workspace: protocol.
  3. Add a peerDependency (like react to package-b)
  4. The resolved paths for package-b will now be virtual and not properly transpiled.

I just came here to say that we have this very setup and I think our modules (package-b in the example above) are properly transpiled, and always have been I think.

I remember that we've seen some strange "virtual" paths in the transpiled output but I believe it was when we used the node-modules linker and hoisting decided to work differently than we expected. With PnP, I don't remember having issues.

But as you describe it, @jpulec, it sounds like there should be an issue so I'll be watching this 😄.

mkotsollaris commented 2 years ago

is there any update on this? I would happily peer program/onboard to the project to find a resolution!

Getting the error as long as I enable nodeLinker: pnp

I think this issue seems to be related.

@borekb @martpie @jpulec

martpie commented 2 years ago

I was a little silent in the past months due to loads of work on my plate, but I'm on holidays so I may have some time to investigate :)

Out of curiosity, can you try what I mentioned here? https://github.com/martpie/next-transpile-modules/issues/75#issuecomment-726001089

mkotsollaris commented 2 years ago

@martpie

Out of curiosity, can you try what I mentioned here? #75 (comment)

Yes, I am getting the same error as if I would if I was using the npm module directly require('ntm.js')[...]:

next-transpile-modules - an unexpected error happened when trying to resolve "@NAME/PKG". Are you sure the name of the module you are trying to transpile is correct, and it has a package.json with a "main" or an "exports" field?

I would say it makes sense, since having a look at the codebase, it seems that node_modules is mentioned several times in the codebase. PNP does not use node_modules, I think this needs to be abstracted according to the type of package manager?

Since yarn PNP would install deps under .yarn/cache/ in .zip format, I think we would potentially need to do the following:

  1. unzip the folder
  2. transpile (like we would do in the 'normal' case)
  3. re-zip the transpiled results

Thoughts?

yaronvhop commented 1 year ago

Any news?

I managed to reproduce the issue myself https://github.com/smulikHakipod/next-yarn-transpile-error .

Need to run this:

https://github.com/smulikHakipod/next-yarn-transpile-error/blob/master/packages/test-c/package.json#L6

goto http://127.0.0.1:3000

And watch transpilation failed.

I dont think "virtual" modules are supposed to be problematic on their own, its fine for Yarn to create a virtual path for a module.

I put some prints in the code in order to check the comparison next-transpile-modules make and it seems to be something like this:

filePath /Users/user/Downloads/next-yarn-test/.yarn/__virtual__/test-bb-virtual-6b51d9b693/1/packages/test-bb/index.ts modulePath /Users/user/Downloads/next-yarn-test/.yarn/__virtual__/test-bb-virtual-eb81ca8611/1/packages/test-bb transpiled false

You can see that test-bb-virtual-eb81ca8611 is not the same as test-bb-virtual-6b51d9b693, I am guessing because some the compile process spawn child processes and each child process get a different module id (just a guess). Yet I have no idea how to solve it.

I hope someone has a clue. Thanks!

davidcalhoun commented 1 year ago

Quick update: with next@12.2.0 and next-transpile-modules@9.0.0 things are working fine here now! I'll update my original comment as well.

yaronvhop commented 1 year ago

Quick update: with next@12.2.0 and next-transpile-modules@9.0.0 things are working fine here now! I'll update my original comment as well.

Are you sure you are talking about the same issue? I attached above a repo that reproduces the issue, with the latest Next and Next Transpile module. Still reproduces. Can you double-check me, please?