[Feature request ] Support pnp Yarn 2 resolution - "Error: Can't find stylesheet to import."

[sdykae]

Like sass-loader,
Support @workspace/module pnp node-module resolution
Similar "vercel/next.js#15753"

[thgh]

I'm curious how that will work. PR welcome 😉

[eagerestwolf]

I find myself in a similar situation, but I'm not entirely sure how it would be addressed. Based on my knowledge, in order to get a file or package when using Yarn PnP, the script in question needs to require(), import(), or import the file/package in question and let the package manager itself respond. Obviously, import() isn't feasible since it's asynchronous; import can only be used at the top level; and require(), while it technically works, is very much blurring the lines between CommonJS and ES Modules.

Even barring that however, Yarn PnP has another problem that currently has a workaround, but the Yarn team has stated is going away eventually. Yarn really doesn't want any package accessing any package or file that it doesn't explicitly list as a dependency. Currently, in the .yarnrc.yml, you can set the pnpMode to loose to allow packages to access files they don't list as dependencies, but Yarn still throws warnings; and, as previously stated, this option is supposed to go away completely at some point in time. Granted, the user can provide packageOverrides in the .yarnrc.yml to explicitly tell Yarn that yes, this package is indeed allowed to access this other package; however, this still requires user intervention.

I honestly find myself at a loss here. I suppose an idea could be to attempt to load file as the plugin is doing now, and then attempt to require() it if it can't be found, instead of trying to access to node_modules directly. The issue then becomes require() is generally used for JavaScript, so I'm not sure how Yarn responds when trying to require() a non-JavaScript file. From a technical perspective, Node (when using require()) returns a handle to a CommonJS module; but Yarn hooks the Node loaders when operating in PnP mode and replaces Node's loaders with its own. Technically speaking, dynamic import() is the solution, but trying to deal with an asynchronous operation in a synchronous thread is a total nightmare.

[eagerestwolf]

Except, I just noticed, the problem isn't with this plugin, it's with sass/node-sass itself. You're passing the concatenated file and include paths directly to the sass compiler, so this is actually a sass issue. You can probably safely close this.

Turns out, node-sass/sass has no concept of where the file is coming from. I will look into this for you, and see about a PR (might even be able to make it in for V3). I just want to verify that this won't break node-sass compatibility because the proposed solution I have in mind works with sass, but I'm not sure about node-sass.

[eagerestwolf]

I have an update. The solution I have in mind will work, but it doesn't work versions of node-sass less than 2.0.0, and it's technically experimental, but it should work with all the existing setups: npm/yarn < 2, yarn >= 2 in nodeLinker mode, and Yarn PnP, but it's also gonna require a little bit of computing power to accomplish. Basically, this block in the renderToCss function:

const css = sass.renderSync(Object.assign({
  data: prefix + scss,
  includePaths
}, options)).css.toString()

needs to change to this:

const css = sass.renderSync(Object.assign({
  data: prefix + scss,
  includePaths,
  importer: (url, prev, done) => {
    const resolved;

    if (fs.existsSync(url)) {
      resolved = url;
    } else {
      const finalUrl = url.startsWith('~') ? url.replace('~', '') : url
      resolved = require.resolve(finalUrl)
    }

    done({ file: resolved })
  }
}, options)).css.toString()

I haven't tested this yet, but the general idea is to hook into libsass's import resolver. If the file exists, we just pass it along as it was. If it doesn't, we try to resolve it using Node. For npm and yarn < 2 this is basically just pulling from the node_modules folder. For yarn >= 2 in node-modules mode, this will pull from the global (or workspace) cache. Finally, for PnP, since Yarn hooks this API from Node, it should return the contents of the file requested...I have not tested this yet though.

Update: I have tested it...and it works! I will rebase, squash and submit a PR. This is the final TypeScript code, sorry for the messiness. I didn't want to type this to either node-sass or sass, so I put the types for the callback in manually. PR is open #83.

const render = sass.renderSync(
  Object.assign(
    {
      data: prefix + scss,
      outFile: dest,
      includePaths,
      importer: (url: string, prev: string, done: (data: { file: string } | { contents: string } | Error | null) => void): { file: string } | { contents: string } | Error | null | void => {
        let resolved;

        if (existsSync(url)) {
          resolved = url;
        } else {
          const finalUrl = url.startsWith('~') ? url.replace('~', '') : url
          resolved = require.resolve(finalUrl)
        }

        return { file: resolved }
      }
    },
    options
  )
)