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

ESBuild unexpectedly sees .pnp.cjs in monorepo project root #3338

Closed
Peter-Sparksuite opened this issue Aug 23, 2023 · 3 comments
Closed

ESBuild unexpectedly sees .pnp.cjs in monorepo project root #3338

Peter-Sparksuite opened this issue Aug 23, 2023 · 3 comments

Comments

@Peter-Sparksuite
Copy link

The situation

I have a monorepo that's using Yarn's Plug'n'Play, and within that, another project that's supposed to be pretty much stand-alone, and indicates this by setting in the local .yarnrc.yml, nodeLinker: node-modules (ie: no Plug'n'Play).

However, when I use a simple ESBuild invocation from within a node script to produce a result 'out.js', an error is appearing which indicates that the monorepo's root .pnp.cjs has been seen, and is being used, which is totally unexpected.

How to reproduce an example of this behavior

I'm facing an issue that on the face of it, seems like perhaps it's an unexpected ESBuild behavior, but, if I'm omitting something, then please chime in, and help me get this set up so that it will work.

I've attached a minimal monorepo project to demonstrate the behavior I'm seeing.

monorepo.tar.gz

The attachment was created using the following command (for those that might be concerned):

% tar -cvzf monorepo.tar.gz monorepo
a monorepo
a monorepo/.yarnrc.yml
a monorepo/index.js
a monorepo/yarn.lock
a monorepo/package.json
a monorepo/esbuild
a monorepo/esbuild/.yarnrc.yml
a monorepo/esbuild/README.md
a monorepo/esbuild/yarn.lock
a monorepo/esbuild/package.json
a monorepo/esbuild/app.js
a monorepo/esbuild/compile.js

To reproduce the problem behavior, follow these steps, assuming you have Yarn and a recent NodeJS installed:

Note: the following assumes you're using some kind of unix-like system:

  1. extract the files using tar -xzvf monorepo.tar.gz
  2. move to the monorepo folder: cd monorepo
  3. set the Yarn version to 'berry': yarn set version berry
  4. let Yarn set things up here: yarn install
  5. at this point, you should find that you have a .pnp.cjs file in the current folder. Verify with: ls -al .pnp.cjs. You should see details of the file being shown.
  6. go to the child folder: cd esbuild
  7. let Yarn set things up here too: yarn install
  8. ask ESBuild to use app.js to produce out.js: node compile.js
  9. at this point, you should be seeing the following error:
% node compile.js
✘ [ERROR] Could not resolve "@cloudflare/kv-asset-handler"

    app.js:1:31:
      1 │ const kvAssetHandler = require('@cloudflare/kv-asset-handler');
        ╵                                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

  The Yarn Plug'n'Play manifest forbids importing "@cloudflare/kv-asset-handler" here because it's
  not listed as a dependency of this package:

    ../.pnp.cjs:48:33:
      48 │           "packageDependencies": [\
         ╵                                  ~~

  You can mark the path "@cloudflare/kv-asset-handler" as external to exclude it from the bundle,
  which will remove this error. You can also surround this "require" call with a try/catch block to
  handle this failure at run-time instead of bundle-time.

/Users/peter/tmpmono/monorepo/esbuild/node_modules/esbuild/lib/main.js:1649
  let error = new Error(text);
              ^

Error: Build failed with 1 error:
app.js:1:31: ERROR: Could not resolve "@cloudflare/kv-asset-handler"
    at failureErrorWithLog (/Users/peter/tmpmono/monorepo/esbuild/node_modules/esbuild/lib/main.js:1649:15)
    at /Users/peter/tmpmono/monorepo/esbuild/node_modules/esbuild/lib/main.js:1058:25
    at /Users/peter/tmpmono/monorepo/esbuild/node_modules/esbuild/lib/main.js:1003:52
    at buildResponseToResult (/Users/peter/tmpmono/monorepo/esbuild/node_modules/esbuild/lib/main.js:1056:7)
    at /Users/peter/tmpmono/monorepo/esbuild/node_modules/esbuild/lib/main.js:1085:16
    at responseCallbacks.<computed> (/Users/peter/tmpmono/monorepo/esbuild/node_modules/esbuild/lib/main.js:703:9)
    at handleIncomingPacket (/Users/peter/tmpmono/monorepo/esbuild/node_modules/esbuild/lib/main.js:762:9)
    at Socket.readFromStdout (/Users/peter/tmpmono/monorepo/esbuild/node_modules/esbuild/lib/main.js:679:7)
    at Socket.emit (node:events:513:28)
    at addChunk (node:internal/streams/readable:324:12) {
  errors: [Getter/Setter],
  warnings: [Getter/Setter]
}

Node.js v18.16.0

Why is this not expected?

The child project (monorepo/esbuild) has a .yarnrc.yml file there that contains:

nodeLinker: node-modules

pnpMode: strict

The nodeLinker: node-modules directive instructs Yarn to not use Plug'n'Play to resolve dependencies.

However, it's clear that regardless of this, it appears that the monorepo-root folder-contained .pnp.cjs file is being seen and processed in some way.

If I now hide the .pnp.cjs (eg: by renaming to .pnp.cjs.hidden), and re-run the command, I do not see the error, and the expected out.js is created successfully.

What do I think it should be doing

I think the resolution of modules should be ignoring monorepo/.pnp.cjs entirely, because the child project has the directive (in monorepo/esbuild/.yarnrc.yml) that indicates it is using node-modules for dependency resolution.

OK, it's entirely possible that there's some magic option somewhere that I've not found yet that addresses this issue. If that's the case, please, enlighten me!

@evanw
Copy link
Owner

evanw commented Aug 23, 2023

I don't have time to look into this at the moment, sorry. But this is similar to another situation that has come up before. You should be able to configure the top-level Yarn to ignore this subfolder. Here's a comment from someone who works on Yarn that says how to do that: #2495 (comment)

@Peter-Sparksuite
Copy link
Author

Peter-Sparksuite commented Aug 25, 2023

@evanw , thank you for your comment! I found, following the link you've provided, a way to address the issue!

I would say that the example needs to be followed fairly closely. ie: root project (with .pnp.cjs), then, some folders later, a project that's part of the main monorepo, then an otherwise empty folder containing the folder that itself contains the isolated project. It seems to need that extra containing folder to work correctly. In my case, there's (currently) only the one non-monorepo project, so it seemed like the containing folder could be omitted, but, I haven't seen a way to do that so far, and still have a generic glob pattern that's specific enough to avoid imposing this on other projects.

ie:

monorepo root folder
| - package.json
| - .pnp.cjs
| - child 1
| - child 2
| - child 3
   | - package.json
   | - .....
   | - container folder (otherwise empty)
        | - non-monorepo project folder
             | - package.json
             | - node_modules

It appears that when the non-monorepo project is resolving modules, it ascends the folder hierarchy, and as it does so, it encounters the monorepo .pnp.cjs, which appears to be informed as to the path of the child-project, and uses a regex to decide to play possum regarding any resources it might have.

@Peter-Sparksuite
Copy link
Author

Peter-Sparksuite commented Aug 25, 2023

Since the existing Yarn-provided mechansim provides a work-around, I will close this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants