Sapper + yarn PnP

[eps1lon]

Is your feature request related to a problem? Please describe.

Usage with yarn v2 (specifically the PnP part) requires quite some extra work.

Describe the solution you'd like

As far as I can tell the @sapper/* usage is responsible for most of the problems. Writing to node_modules/ is discouraged in yarn v2.

Describe alternatives you've considered I've worked around the current issues by:

• unplugging sapper and svelte (this might be the easiest issue to fix since we "just" need to adjust the output path of @sapper/)
• adding a custom rollup resolve plugin that maps @svelte/* to src/node_modules/@sapper/*

https://github.com/eps1lon/berry-sapper/compare/ccdab33a687cd96014343ed0cf32484add9f7ccc...a64171dae36b94806e6d99833d65f7668b52db45

How important is this feature to you? yarn v2 isn't released as stable. I don't know if the PnP semantics are subject to change. I made it work for now but the additional setup seems very brittle.

Additional context

Unplugging a package means that it will be unpacked within their own directory, which we don't recommend as it breaks zero-installs):

-- https://github.com/storybookjs/storybook/issues/6899#issuecomment-496909337

[kylecordes]

Regardless of yarn though, writing into node_modules is at best highly optimistic. It should be quite strongly discouraged.

[Conduitry]

Sapper doesn't write into node_modules/@sapper; it writes into src/node_modules/@sapper, where nothing else is going to be installed.

I don't know what Yarn is doing, or how what it's doing affects module resolution during bundling, but if Yarn is turning its back on the precedent of Node resolution rules, it's expected that things will break, and I don't think this is Sapper's problem currently.

[Rich-Harris]

Can anyone give context on why Yarn is ignoring/breaking the Node resolution algorithm? Sapper isn't relying on some quirky edge case behaviour here, this is how node_modules is meant to work

[Rich-Harris]

jinx

[Rich-Harris]

cc @arcanis 😀

[eps1lon]

Can anyone give context on why Yarn is ignoring/breaking the Node resolution algorithm?

I guess https://yarnpkg.com/lang/en/docs/pnp/ is a good primer?

[Conduitry]

This seems to be the dangerous assumption, mentioned in those docs:

When you think about it, Yarn knows everything about your dependency tree - it evens installs it!

I was wondering about how your project was resolving regular dependencies at all, and then I saw there's a replacement rollup-plugin-pnp-resolve plugin. Having a one-off plugin that first resolves modules beginning with @sapper/ seems reasonable. The could probably be done in a less brittle way by not hardcoding specific filenames to occur after @sapper/, but honestly your current implementation doesn't seem that brittle.

I still don't understand why svelte and sapper would need to be installed specially, or what 'unplugging' means here.

[Rich-Harris]

The irony here is that Yarn PnP and Sapper (and Svelte) are all doing the same thing — minimising the amount of work that happens at runtime by generating information/code at compile time

[arcanis]

Hey! I'd be happy to help, but just remember that I'm discovering sapper at the same time so the more context you can share the best it is! Stacktraces, reproduction scripts, I take it all 🙂

edit It's a lot of text because I wanted to give as much info as possible to understand what we're doing, sorry about the wall of text ... 😕

To give you some context of my own, we introduced about a year ago Plug'n'Play. This install mode offers a lot of advantages over the regular node_modules (more details in the rfc and this overview), but one critical part is: Yarn takes ownership of the bare identifiers resolution. Node doesn't have to "try and repeat" to figure out where to load a package: Yarn knows that if package A require package B, then it's [email protected], and that [email protected] is installed in one place.

Now you might wonder what are the guarantees in term of compatibility. Well, everything that doesn't specifically rely on the node_modules folders existing works! That's actually a lot of things: require works, of course, require.resolve as well, yarn run, yarn <binary>, ... the only things that break are usually discouraged patterns such as requiring a package that's only available through hoisting, or crossing the node_modules boundaries.

Anyway, it worked really well, and we've worked with the ecosystem to fix the problems PnP highlighted (a lot of packages had missing dependencies, PnP detected it, and we fixed that with the help of their various maintainers). It's still not flawless, but it's good enough that it'll be the default install mode for Yarn 2, that's currently in development.

Something else that we'll ship with Yarn 2 is called "Zip Loading". The idea is simple: instead of loading the vendors from the node_modules (or the cache with PnP), we load them instead from the package archives - similar to what PHP does with phar, or Electron with ASAR. That's why any write to the package directory itself will fail (such as writing into ${__dirname}/foo.txt from a vendor, for example).

In this context, "unplugging" means that the "unplugged" package will be extracted on the disk, like before. It's kind of a workaround for the packages that cannot work from within a zip archive - for example the native packages, but also those that contain shellscripts, or postinstall scripts, etc.

[Rich-Harris]

@arcanis thank you! So, here's the context: Sapper is a framework like Next.js that allows you to build large complex apps without all the usual plumbing. Part of how it does that is by looking at your folder structure (your app will have a src/routes folder which maps to the web app's routes) and generating a bespoke app based on that.

This means that when you call (for example) the start function, it knows which JS/CSS bundles to load for the initial route (wherever the user has landed), because it's looking at a pre-generated manifest that is baked into your app.

In earlier versions, your client app would typically look like this:

import * as sapper from '../__sapper__/client.js';

sapper.start({
  target: document.querySelector('#sapper')
});

In other words, it was being written to [project]/__sapper__. That's ugly, but it's also very inconvenient when you're importing Sapper functions (like goto) from deeply nested modules, because you end up with the ../../../ problem.

In newer versions, the generated app gets written to [project]/src/node_modules. This has the convenience of using a regular package, but retains Sapper's unique benefits.

What would be the recommended move for Yarn 2 users building Sapper apps?

[arcanis]

This means that when you call (for example) the start function, it knows which JS/CSS bundles to load for the initial route (wherever the user has landed), because it's looking at a pre-generated manifest that is baked into your app.

Ahah yeah it reminds me something 😄

What would be the recommended move for Yarn 2 users building Sapper apps?

From what you described it should mostly work without needing any unplug since you're writing into the project folder rather than your own 🤔 but if we assume that there is indeed a problem there, my first guess would be that the project root is incorrectly detected as being sapper instead of the project (which sometimes happen depending on the heuristics). Maybe @eps1lon can investigate to figure out where are the FS calls and what is passed to them?

Something else is that src/node_modules won't be available by default (since it's unknown to Yarn's dependency). To make it work I'd suggest to add a link: dependency to the package.json - something like this:

{
  "dependencies": {
    "app": "link:./src/node_modules"
  }
}

This will cause Yarn to resolve app/foo/index.js to ./src/node_modules/foo/index.js, which seems to match the effect you want to achieve.

[Rich-Harris]

Ah ok, it sounds like we'd be able to get this to work by updating sapper-template with the following:

{
  "dependencies": {
    "@sapper": "link:./src/node_modules/@sapper"
  }
}

@eps1lon could you confirm if that works?

[eps1lon]

@eps1lon could you confirm if that works?

"devDependencies": {
    "@sapper/app": "link:./src/node_modules/@sapper/app",
    "@sapper/server": "link:./src/node_modules/@sapper/server",
    "@sapper/service-worker": "link:./src/node_modules/@sapper/service-worker"
}

works: https://github.com/eps1lon/berry-sapper/pull/1

Just using @sapper caused package is trying to access another package without the second one being listed as a dependency of the first one. devDependencies so that it will be picked up as external.

[arcanis]

Awesome! That's interesting - @sapper isn't a valid package name so it should have been rejected outright 😄 btw, if any of those generated packages has its own dependencies, then it would be portal: instead of link: (Yarn 2 only).

[eps1lon]

@sapper isn't a valid package name so it should have been rejected outright smile btw,

I think I added it manually to the package.json.

sapper and svelte still need to be unplugged for yarn dev to work (build and export work). Otherwise I get

✗ client
Failed to build — error in /home/eps1lon/Development/src/js/berry-sapper/.yarn/cache/svelte-npm-3.6.9-e3f62e0110f75f256d4ee3b624f27f9118cf8849d854db3b40b3bed9685d50ce.zip/node_modules/svelte/index.mjs: ENOTDIR: not a directory, watch '/home/eps1lon/Development/src/js/berry-sapper/.yarn/cache/svelte-npm-3.6.9-e3f62e0110f75f256d4ee3b624f27f9118cf8849d854db3b40b3bed9685d50ce.zip/node_modules/svelte/index.mjs'
✗ server
Failed to build — error in /home/eps1lon/Development/src/js/berry-sapper/.yarn/cache/svelte-npm-3.6.9-e3f62e0110f75f256d4ee3b624f27f9118cf8849d854db3b40b3bed9685d50ce.zip/node_modules/svelte/store/index.mjs: ENOTDIR: not a directory, watch '/home/eps1lon/Development/src/js/berry-sapper/.yarn/cache/svelte-npm-3.6.9-e3f62e0110f75f256d4ee3b624f27f9118cf8849d854db3b40b3bed9685d50ce.zip/node_modules/svelte/store/index.mjs'

[arcanis]

That makes sense - maybe the vendor files should be excluded from the watch, since the paths will change if the files change? Although I guess in node_modules installs the files may change while the path stays the same 🤔

[Rich-Harris]

@sapper isn't a valid package name so it should have been rejected outright

hmm... I wonder if we need to change it to a valid package name instead. We could change it to app (there is a package with that name, but I don't anticipate conflicts)

Not sure I understand the thing about yarn dev needing sapper and svelte to be unplugged? They're just regular devDependencies, no?

[arcanis]

Not sure I understand the thing about yarn dev needing sapper and svelte to be unplugged? They're just regular devDependencies, no?

From what I gather you have a watcher system that tries to watch the following file (notice the .zip):

/.../berry-sapper/.yarn/cache/svelte-npm-3.6.9.zip/node_modules/svelte/index.mjs

Since it doesn't exist, a ENOTDIR error is thrown. When svelte is unplugged it becomes accessible:

/.../berry-sapper/.yarn/unplugged/svelte-npm-3.6.9/node_modules/svelte/index.mjs

Now, maybe our fs layer should just transform the watch call into a noop for the vendored files, but watchers are a fickle beast and it won't be easy .. I'll think about it 🤔

[Rich-Harris]

That doesn't sound like a Sapper-specific issue, that sounds like something that would affect every app built with Rollup (which watches all your dependencies, and doesn't stop at node_modules, since that makes npm link workflows nightmarish)

[arcanis]

I've submitted https://github.com/yarnpkg/berry/pull/315 which should mitigate the issue (fs.watch becomes a noop on vendor files - only those stored within zip archives are affected).

[thealjey]

Ah ok, it sounds like we'd be able to get this to work by updating sapper-template with the following:

{
  "dependencies": {
    "@sapper": "link:./src/node_modules/@sapper"
  }
}

@eps1lon could you confirm if that works?

I think this would've worked if:

1. the name of a package did not begin with an illegal character
2. there was a package.json file in the package directory

[felixakiragreen]

I can't get sapper to work with yarn v2. Going to roll back to v1.

I've tried linking directly to the files:

"@sapper/app": "link:./src/node_modules/@sapper/app.mjs",
"@sapper/server": "link:./src/node_modules/@sapper/server.mjs",
"@sapper/service-worker": "link:./src/node_modules/@sapper/service-worker",

Note .mjs must include the extension otherwise results in this error:

Error: Couldn't find a suitable Node resolution for the specified unqualified path

Source path: ~/code/website/src/node_modules/@sapper/server
Rejected resolution: ~/code/website/src/node_modules/@sapper/server
Rejected resolution: ~/code/website/src/node_modules/@sapper/server.js
Rejected resolution: ~/code/website/src/node_modules/@sapper/server.json
Rejected resolution: ~/code/website/src/node_modules/@sapper/server.node

I am stuck with this error:

internal/modules/cjs/loader.js:998
    throw new ERR_REQUIRE_ESM(filename);
    ^

Error [ERR_REQUIRE_ESM]: Must use import to load ES Module: ~/code/website/src/node_modules/@sapper/server.mjs
    at Module.load (internal/modules/cjs/loader.js:998:11)
    at Function.module_1.Module._load (~/code/website/.pnp.js:24702:14)
    at Module.require (internal/modules/cjs/loader.js:1040:19)
    at require (internal/modules/cjs/helpers.js:72:18)
    at Object.<anonymous> (~/code/website/__sapper__/dev/server/server.js:8:14)
    at Module._compile (internal/modules/cjs/loader.js:1151:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1171:10)
    at Module.load (internal/modules/cjs/loader.js:1000:32)
    at Function.module_1.Module._load (~/code/website/.pnp.js:24702:14)
    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:71:12) {
  code: 'ERR_REQUIRE_ESM'
}
> Server crashed

Nothing I do seems to change anything since __sapper__/dev/server/server.js is a generated file, and transforms all imports to requires. So... yeah.

Also just doing "@sapper": "link:./src/node_modules/@sapper" doesn't work because the modules can't be resolved.

[arcanis]

@felixakiragreen the fix for that can be found at https://github.com/sveltejs/sapper-template/pull/201. Basically you need to use aliases to make sure you can point the client module resolution to packages that aren't packages.

[mbrowne]

In the meantime, if anyone wants to use yarn 2 and still have it "just work", then add this to your .yarnrc.yml file:

nodeLinker: "node-modules"

That tells it to store modules in a local node_modules folder like yarn 1 and npm do.