[Feature] Support yarn berry/PnP

[eigilsagafos]

In our pursuit to speed up our development flow and CI/CD esbuild has been amazing. Another tool I've been testing a few times is yarn berry (yarn 2.0). As far as I have been able to figure out there are currently two issues with using esbuild with berry.

• yarn berry does not support non-JS binaries ([Bug] SyntaxError when running a "bin" file that isn't authored in JS yarnpkg/berry#882)
• esbuild does not work with yarn PnP

Any thoughts on making esbuild work natively with berry?

[evanw]

It looks like the second part is a duplicate of #154. See that issue for my thoughts. I plan to start experimenting with resolve plugins in esbuild soon FWIW.

For the first part, it sounds like your saying if you install esbuild with yarn, you are unable to call the JavaScript API from a module loaded by PnP because Yarn breaks with all binary files. Is that right? Can you provide example code to reproduce the issue?

[eigilsagafos]

Sorry for not searching through the closed issues.
For the first one you can run yarn esbuild from this repo: https://github.com/eigilsagafos/esbuild-yarn-berry
For the second one, thanks! I'll look forward to trying that out once it lands.

[evanw]

Just took a look. It looks like you want to use the yarn command as a way to invoke the esbuild executable from the command line. However, the executable is deliberately a native binary instead of a JavaScript file to avoid the unnecessary overhead of starting another node process just to run a single executable and exit.

I read through yarnpkg/berry#882 but I don't understand the cross-platform argument. The esbuild package works fine in cross-platform scenarios because it downloads the correct binary for the current platform during the install. This looks like a bug with yarn to me, not with esbuild.

I can think of several workarounds for this bug:

• Use $(yarn bin esbuild) [options] instead of yarn esbuild [options]
• Call esbuild's build() command from a JavaScript file like this, then add a package.json script for that
• Create a JavaScript wrapper package that forwards to the esbuild binary (if you need this workaround in package form)

[sod]

[...] The esbuild package works fine in cross-platform scenarios because it downloads the correct binary for the current platform during the install [...]

One of berry's features is, that you commit the install to git. So you don't even run yarn install anymore after git clone. It runs out of the box. I guess binaries don't fit well in that model.

see https://yarnpkg.com/features/zero-installs

[evanw]

One of berry's features is, that you commit the install to git. So you don't even run yarn install anymore after git clone. It runs out of the box. I guess binaries don't fit well in that model.

Ah, I see. Well if you want a cross-platform esbuild package you should install esbuild-wasm instead. Normally I don’t encourage people to install it because it’s a lot slower but if you need to use a single package for all platforms then it’s pretty much your only option (I’m assuming you don’t want to simultaneously install all 8 esbuild native packages for all supported platforms).

Luckily esbuild is more than 10x faster than other tools, so even though esbuild-wasm is 10x slower than native esbuild, I’m guessing it’s still faster than other tools. You might want to check the performance for your use case yourself though.

Also keep in mind that checking in a huge binary into git and then repeatedly updating it over time might not be the best for git repo size depending on your use case.

[eigilsagafos]

Using the wasm might be a good option in this case, agree. I know the yarn maintainers have been in contact with github about the possible implications that PnP will have when checking in the zip file of every dependency (and updating these on a regular basis). It seemed to be fine. In our case we see that the benefits of berry far outweigh the storage issue. But adding all 8 esbuild binaries does sound like a lot of unnecessary weight.

[evanw]

FYI in case you aren't following along with #111: I'm currently working on the plugin system and keeping this use case in mind. This work is being done on the plugins branch.

Here's a simple plugin using the work-in-progress API that appears to add support for Yarn's PnP API:

let { PosixFS, ZipOpenFS } = require(`@yarnpkg/fslib`)
let libzip = require(`@yarnpkg/libzip`).getLibzipSync()
let zipOpenFs = new ZipOpenFS({ libzip })
let crossFs = new PosixFS(zipOpenFs)
let pnpapi = require('pnpapi')

// More info: https://classic.yarnpkg.com/en/docs/pnp/
let pnpYarnPlugin = plugin => {
  plugin.setName('pnp')
  plugin.addResolver({ filter: /.*/ }, args => {
    return { path: pnpapi.resolveRequest(args.path, args.importDir + '/') }
  })
  plugin.addLoader({ filter: /.*/ }, args => {
    return { contents: crossFs.readFileSync(args.path), loader: 'default' }
  })
}

I say "appears to" because I don't personally use Yarn so I'm not familiar with it. Everything seems to be working but there may be edge cases I'm not aware of. In any case I think it at least shows that the plugin system should be able to solve this, even if the final Yarn PnP API calls are a little different.

[eigilsagafos]

Thanks @evanw for taking the time to test this with the plugin system! Will have a look at this soon.

[eigilsagafos]

@evanw I finally got around to testing esbuild plugins api with PnP. It looks very promising! One issue I had to "work around" was how to handle external. My assumption was that any library passed as external to esbuild would just be ignored by plugins. That was not the case. Maybe there could be an option to include externals int the addResolver options?

[merceyz]

FYI in case you aren't following along with #111: I'm currently working on the plugin system and keeping this use case in mind. This work is being done on the plugins branch.

Here's a simple plugin using the work-in-progress API that appears to add support for Yarn's PnP API:

let { PosixFS, ZipOpenFS } = require(`@yarnpkg/fslib`)
let libzip = require(`@yarnpkg/libzip`).getLibzipSync()
let zipOpenFs = new ZipOpenFS({ libzip })
let crossFs = new PosixFS(zipOpenFs)
let pnpapi = require('pnpapi')

// More info: https://classic.yarnpkg.com/en/docs/pnp/
let pnpYarnPlugin = plugin => {
  plugin.setName('pnp')
  plugin.addResolver({ filter: /.*/ }, args => {
    return { path: pnpapi.resolveRequest(args.path, args.importDir + '/') }
  })
  plugin.addLoader({ filter: /.*/ }, args => {
    return { contents: crossFs.readFileSync(args.path), loader: 'default' }
  })
}

I say "appears to" because I don't personally use Yarn so I'm not familiar with it. Everything seems to be working but there may be edge cases I'm not aware of. In any case I think it at least shows that the plugin system should be able to solve this, even if the final Yarn PnP API calls are a little different.

@evanw Just a note on that plugin, it's missing the VirtualFS so it wont be able to handle peer dependencies
https://github.com/yarnpkg/berry/blob/7fcd174e610c3ebdd60391f1b915eff7f7f51431/packages/yarnpkg-pnp/sources/loader/_entryPoint.ts#L30-L37

[eigilsagafos]

If anyone is interested in testing a plugin with pnp support: https://www.npmjs.com/package/esbuild-plugin-pnp
Feedback/issues is welcome.

[merceyz]

You don't actually have to use the pnpapi nor any of the Yarn packages directly for this, a simple require.resolve(args.path, {paths:[args.importDir]}) and require('fs').readFileSync(args.path) should be enough.

[evanw]

Closing this because it should now be possible to build this with the plugin API. As mentioned above, something like this may be sufficient:

let yarnPnpPlugin = {
  name: 'yarn-pnp',
  setup(build) {
    build.onResolve({ filter: /.*/ }, args => ({
      path: require.resolve(args.path, {paths: [args.resolveDir]}),
    }))
    build.onLoad({ filter: /.*/ }, async (args) => ({
      contents: await require('fs').promises.readFile(args.path),
      loader: 'default',
    }))
  },
}

[merceyz]

@evanw I don't suppose you're willing to automatically register a plugin like that if you detect PnP? Totally understandable if you don't

[evanw]

@evanw I don't suppose you're willing to automatically register a plugin like that if you detect PnP? Totally understandable if you don't

I'm not planning to do that. Yarn PnP is a very invasive change to node that changes a fundamental assumption about how it works, so I think it will at best only sort of work with esbuild. The Go world won't know about the monkey-patching Yarn PnP does, as well as binary plugins when they are supported in the future. So there will likely always be things about Yarn PnP that don't work with esbuild.

I expect people to need to do custom tweaks to the path resolution logic to get it to work correctly in a real code base and I don't think it's appropriate for esbuild to try to handle this automatically. For example, the above plugin may need to also modify require.extensions to add implicit extensions such as .ts but doing so means mutating a global, which probably isn't appropriate for esbuild's API to do.

[dmattia]

Just some resolution to this closed issue in case anyone else stops by, yarn now officially supports a esbuild plugin for pnp that they use internally to build yarn itself: https://github.com/yarnpkg/berry/tree/master/packages/esbuild-plugin-pnp

[evanw]

FYI: Native support for PnP has been added to esbuild: https://github.com/evanw/esbuild/releases/tag/v0.15.0 (it became possible to implement this now that the PnP data format has been documented: https://yarnpkg.com/advanced/pnp-spec/).