-
-
Notifications
You must be signed in to change notification settings - Fork 85
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
Pnp workspaces aren't transpiled (yarn berry) #75
Comments
Hello 👋
Can you provide one? I don't have experience with pnp. |
Sure. I've created one. Follow readme to get the error. https://github.com/goszczynskip/next-transpile-modules-bug-75 It looks that yarn changes package paths underneath to local ones. I've tried to add all workspaces as aliases in webpack config but this forces me to include them in package.json of application. I'd rather avoid it because it pollutes package.json's dependencies. |
Any updates here? Known workaround? |
There is a PR opened, but its creator needs to update it in order for the tests to be ran against it. Also, integrations tests would be welcome. |
It's not a lot of work left, but it needs to be done and I do not have time to invest on a setup I don't know too well sorry. So a branch fork + new PR is more than welcome! |
I'll try to update it today. EDIT: |
Should be fixed in 3.3.0 🚀 |
I am MacOS and still having this issue. I cloned @goszczynskip 's repo and updated |
@martpie Also attempted to use this in our monorepo with Yarn 2 pnp. Upgraded to version |
Also attempted to transpile a simple repo on the I have little idea of the inner workings of WebPack, so please ignore this comment if it is ignorant..! It seems that this package hijacks the WebPack node modules resolution process by using a custom regex pattern based upon the modules a developer has passed into the package config? From my understanding webpack module resolution is a superset of the node modules resolution api called "enhanced-resolve". The reason this package does not support yarn pnp is that it's looking for a Here comes my ignorance... Would it not be better to utilise WebPack's built in "enhanced-resolve" algorithm that includes PnP support to resolve the modules? It can be imported as a package: https://github.com/webpack/enhanced-resolve. const fs = require("fs");
const { CachedInputFileSystem, ResolverFactory } = require("enhanced-resolve");
// create a resolver
const nextModulesResolver = ResolverFactory.createResolver({ ...options });
nextModulesResolver.resolve(...); PnP compatibility table: https://yarnpkg.com/features/pnp#compatibility-table |
To anyone facing this issue, I currently unfortunately have no time to work on this before one or two weeks (I also never used yarn 2), but I'd be more than willing to accept PRs 👍 The first step to take is to create a failing test, so we can check easily if it's fixed or not when working on it.
I also plan to refactor the whole library at some point in the next months as it grew a bit out of hand, it's hard to test, and complex regexes are more confusing than helping. Hope it helps 👍 |
Amazing! I also dont have time in the next couple of sprints. Although after that can contribute for sure. |
@martpie Here is some context regarding "enhance-resolve" and PnP. This is a conversation between Yarn guys and Webpack guys: webpack/enhanced-resolve#162 If I could be so bold as to make some suggestions... I believe the best approach would be to create a Key take aways would be:
|
For those wanting a solution asap I created a custom next webpack plugin that does the following and it works All our workspace pakages start with config.module.rules.push({
test: /\.(tsx|ts|js|mjs|jsx)$/,
include: (input) => {
if (input.includes('@pkg')) {
return true;
}
return false;
},
use: options.defaultLoaders.babel
}); |
Yes, this should do the trick indeed (hmr won't work unfortunately), be careful that with this solution, if you use |
yep certainly a hack, not a long term solution. |
@martpie I've tried to create failing tests for yarn2 in current files layout. Unfortunately I haven't found easy way to make it work (without rewriting tests bootstraping code). Yarn 2 doesn't allow nested packages that aren't part of parent workspaces. We can overcome this limitation by running tests inside of Docker image. |
Using a specific path seems to work for us: - withTM(['@scope/lib']
+ withTM(['../lib'] It's uglier but works. |
I was able to get it working using the following hack that seems more general than the have proposed above by @andrewmclagan
Is this something we could include in next-transpile-modules? |
@ukarlsson @andrewmclagan is this something you use instead of next-transpile-modules, or together with it? |
@ukarlsson thank you for diving into it and providing a solution! There is #115 where I try to keep track of all the things I would like to do for |
@borekb I use it instead of next-transpile-modules, just putting that directly into |
Note that hot-reloading will probably not work ;) (that's also one the things |
Sidenote: It looks like the solution provided by @ukarlsson transpiles ALL the depedencies, which must be quite bad for build performances, so I'd recommend to restrict it to your packages only ;) |
The idea with the solution I provided above was to exclude the dependencies handled by Yarn, therefore it includes aforementioned |
@martpie do you think that supporting PnP is a relatively large overall change in the source code, or do you think it's a small fix somewhere? I'd be willing to look at it closer but would like to know what I'm subscribing for 😄. |
Reference for myself: Basically, |
Yep, as you already described above. In our app, when we use PnP and scoped packages, the error during
I.e., there are various |
Wow 😲 that a lot. Congrats I guess?! 😄 |
Hello there! 👋 Could someone test the branch
|
@martpie I tried the branch. BTW, this seems to be the correct command, at least for Yarn 2: yarn add next-transpile-modules@martpie/next-transpile-modules#nextjs-10
# Effectively the same as:
yarn add next-transpile-modules@https://github.com/martpie/next-transpile-modules.git#commit=79375a68b0a21c7eb234cf81048144d8f22491df It produces this error: $ next build
> Build error occurred
Error: next-transpile-modules tried to access resolve, but it isn't declared in its dependencies; this makes the require call ambiguous and unsound.
Required package: resolve (via "resolve")
Required by: next-transpile-modules@https://github.com/martpie/next-transpile-modules.git#commit=79375a68b0a21c7eb234cf81048144d8f22491df (via /Users/borekb/dev/company/project/packages/.yarn/cache/next-transpile-modules-https-998e3c593d-2cef455db2.zip/node_modules/next-transpile-modules/src/)
Require stack:
- /Users/borekb/dev/company/project/packages/.yarn/cache/next-transpile-modules-https-998e3c593d-2cef455db2.zip/node_modules/next-transpile-modules/src/next-transpile-modules.js
- /Users/borekb/dev/company/project/packages/citygolf/next.config.js
- /Users/borekb/dev/company/project/packages/.yarn/$$virtual/next-virtual-e93fc6ab86/0/cache/next-npm-9.5.2-504d9b0249-bcd1379ae6.zip/node_modules/next/dist/next-server/server/config.js
- /Users/borekb/dev/company/project/packages/.yarn/$$virtual/next-virtual-e93fc6ab86/0/cache/next-npm-9.5.2-504d9b0249-bcd1379ae6.zip/node_modules/next/dist/build/index.js
- /Users/borekb/dev/company/project/packages/.yarn/$$virtual/next-virtual-e93fc6ab86/0/cache/next-npm-9.5.2-504d9b0249-bcd1379ae6.zip/node_modules/next/dist/cli/next-build.js
- /Users/borekb/dev/company/project/packages/.yarn/$$virtual/next-virtual-e93fc6ab86/0/cache/next-npm-9.5.2-504d9b0249-bcd1379ae6.zip/node_modules/next/dist/bin/next
at internalTools_makeError (/Users/borekb/dev/company/project/packages/.pnp.js:29690:34)
at resolveToUnqualified (/Users/borekb/dev/company/project/packages/.pnp.js:30648:23)
at resolveRequest (/Users/borekb/dev/company/project/packages/.pnp.js:30740:29)
at Object.resolveRequest (/Users/borekb/dev/company/project/packages/.pnp.js:30806:26)
at Function.external_module_.Module._resolveFilename (/Users/borekb/dev/company/project/packages/.pnp.js:29923:34)
at Function.external_module_.Module._load (/Users/borekb/dev/company/project/packages/.pnp.js:29788:48)
at Module.require (internal/modules/cjs/loader.js:1025:19)
at require (internal/modules/cjs/helpers.js:72:18)
at Object.<anonymous> (/Users/borekb/dev/company/project/packages/.yarn/cache/next-transpile-modules-https-998e3c593d-2cef455db2.zip/node_modules/next-transpile-modules/src/next-transpile-modules.js:2:17)
at Module._compile (internal/modules/cjs/loader.js:1137:30) |
Thank you very much 🙌 I've fixed it, could you try again? 😄 (8c9e47d) |
This time, I get this: $ yarn build
> Build error occurred
Error: Cannot find module '@company/lib' from '/Users/borekb/dev/company/project/packages/.yarn/cache/next-transpile-modules-https-31f28f89ec-8d20c5cde4.zip/node_modules/next-transpile-modules/src'
at Function.resolveSync [as sync] (/Users/borekb/dev/company/project/packages/.yarn/cache/resolve-patch-46f4fba2f6-8ad4c7742f.zip/node_modules/resolve/lib/sync.js:90:15)
at /Users/borekb/dev/company/project/packages/.yarn/cache/next-transpile-modules-https-31f28f89ec-8d20c5cde4.zip/node_modules/next-transpile-modules/src/next-transpile-modules.js:35:32
at Array.map (<anonymous>)
at generateResolvedModules (/Users/borekb/dev/company/project/packages/.yarn/cache/next-transpile-modules-https-31f28f89ec-8d20c5cde4.zip/node_modules/next-transpile-modules/src/next-transpile-modules.js:34:6)
at withTM (/Users/borekb/dev/company/project/packages/.yarn/cache/next-transpile-modules-https-31f28f89ec-8d20c5cde4.zip/node_modules/next-transpile-modules/src/next-transpile-modules.js:61:29)
at /Users/borekb/dev/company/project/packages/.yarn/cache/next-compose-plugins-npm-2.2.0-daa8cb9862-f27d897f26.zip/node_modules/next-compose-plugins/lib/compose.js:100:23
at Array.forEach (<anonymous>)
at composePlugins (/Users/borekb/dev/company/project/packages/.yarn/cache/next-compose-plugins-npm-2.2.0-daa8cb9862-f27d897f26.zip/node_modules/next-compose-plugins/lib/compose.js:77:11)
at /Users/borekb/dev/company/project/packages/.yarn/cache/next-compose-plugins-npm-2.2.0-daa8cb9862-f27d897f26.zip/node_modules/next-compose-plugins/lib/index.js:22:38
at normalizeConfig (/Users/borekb/dev/company/project/packages/.yarn/$$virtual/next-virtual-17b8ed4149/0/cache/next-npm-9.5.2-504d9b0249-bcd1379ae6.zip/node_modules/next/dist/next-server/server/config.js:5:1990) {
code: 'MODULE_NOT_FOUND'
} Do you have a PnP project you're testing it on? Maybe it's somehow different but I think our Yarn workspaces setup is pretty standard. |
Thank you very much for helping me testing this. I will setup a local project in order to debug that. I tried to do test automation, but berry does not like to find I'll keep you in touch |
I'd be happy to prepare a public repo for you but if you're already on it, maybe no need?! Let me know if I can help. |
Can you confirm that using ntm directly in your project (without using it from (if you copy paste the following in your project: https://raw.githubusercontent.com/martpie/next-transpile-modules/nextjs-10/src/next-transpile-modules.js) const withTM = require('./ntm')([/* whatever */]); |
@martpie As I said earlier creating automatic tests with current architecture isn't possible. I can prepare yarn2 testing but that will require from us to change approach to use docker images for testing. Inside each image I can prepare target env (npm, yarn1, yarn2) and we'll run tests inside containers. |
@goszczynskip Thank you very much, I just created a small reproduction setup on my laptop so I can quickly prototype a solution. It seems so far that v5 will support berry fine, but there is just a module resolution issue left, and I am trying to pinpoint the root cause: apparently, when you copy-paste the updated plugin to your codebase, things work fine now, but so we're almost there! |
I'm so sorry, I forgot to mention you need to install
so sorry for all the back and forths |
No problem 😄. With that, I'm getting this error. In other words, I don't see any difference between installing n-t-m via This is roughly my const withPlugins = require('next-compose-plugins');
const withTM = require('./ntm');
module.exports = withPlugins(
[
withTM(['@company/lib']),
],
{
env: {
},
typescript: {
ignoreDevErrors: true,
},
webpack: (config) => {
// etc.
return config;
},
}
); |
Ok, then your problem is probably in your |
Oh interesting, we don't have any main file (i.e., our lib doesn't have import { Xyz } from '@company/lib/components/Xyz'; This works fine for us without PnP, and even with PnP if I make this change to the const withPlugins = require('next-compose-plugins');
const withTM = require('./ntm');
module.exports = withPlugins(
[
- withTM(['@company/lib']),
+ withTM(['../lib']),
],
{
webpack: (config) => {
// etc.
},
}
); I'd like to avoid the |
Would be interesting if you can try to add a |
Oh yeah that works! 🎉 That's a good-enough workaround for us, I guess n-t-m should work even without it but it's a great tip, thank you! |
Great, at least that's one point solved :) I will investigate the two things, and when we're ready, I can probably release the v5:
thank you very much for your time! |
You can also transpile both paths so see what main and module go to. Right now i point to empty files when it doesn't matter (just to make compile pass and resolve) this release seems to work for me and I've PRd it back to this project, but if anyone it stuck - this might work in the interm
|
@martpie Can I use just the company scope without a specific package name? For example, this works: |
With |
Exciting! 🎉 Any of the current branches implement that so that I could try? |
Is anyone willing to try the branch again? I tested a yarn berry and managed to fix the resolution issue (without having to copy paste the library in your project). You can also try
|
Are you trying to transpile a local package or an npm package?
Local package
Describe the bug
I'm trying to make transpilation work with yarn berry. I have multiple workspaces in my repo and listing them as an configuration parameter doesn't trigger transpilation.
My config:
Import in one of pages:
Error in console:
To Reproduce
Expected behavior
It should be transpiled
Setup
next-transpile-modules
version: 3.1.0npm
/yarn
version: 2.0.0-rc.30The text was updated successfully, but these errors were encountered: