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

[Bug] URL instances are not supported by the FS layer #899

Closed
1 task
demurgos opened this issue Feb 5, 2020 · 3 comments · Fixed by #2291
Closed
1 task

[Bug] URL instances are not supported by the FS layer #899

demurgos opened this issue Feb 5, 2020 · 3 comments · Fixed by #2291
Labels
bug Something isn't working reproducible This issue can be successfully reproduced

Comments

@demurgos
Copy link
Contributor

demurgos commented Feb 5, 2020

  • I'd be willing to implement a fix

Describe the bug

The virtual FS layer does not support URL instances for files. Passing a URL causes an internal error as it tries to treat it as a string. This diverges from Node's FS API and breaks packages identifying files through URL instances.

To Reproduce

Full reproduction repository: https://github.com/demurgos/yarn-fs-url-bug

const {writeFileSync} = require("fs");
const {resolve} = require("path");
const {pathToFileURL} = require("url");

const outUrl = pathToFileURL(resolve("foo.txt"));
expect(() => writeFileSync(outUrl, "Hello, World!")).not.toThrow();

Here is the error I get:

$ yarn start
/data/projects/web/yarn-fs-url-bug/.pnp.js:6182
    const match = p.match(VIRTUAL_REGEXP);
                    ^

TypeError: p.match is not a function
    at Function.resolveVirtual (/data/projects/web/yarn-fs-url-bug/.pnp.js:6182:21)
    at VirtualFS.mapToBase (/data/projects/web/yarn-fs-url-bug/.pnp.js:6215:22)
    at VirtualFS.fsMapToBase (/data/projects/web/yarn-fs-url-bug/.pnp.js:1231:19)
    at VirtualFS.writeFilePromise (/data/projects/web/yarn-fs-url-bug/.pnp.js:1133:46)
    at PosixFS.writeFilePromise (/data/projects/web/yarn-fs-url-bug/.pnp.js:1133:24)
    at /data/projects/web/yarn-fs-url-bug/.pnp.js:1360:9
    at processTicksAndRejections (internal/process/task_queues.js:79:11)

Screenshots

N/A

Environment if relevant (please complete the following information):

  • OS: Linux
  • Node version: 13.7
  • Yarn version: 2.0.0-rc.27

Additional context

Node supports passing URL instances to the fs API since Node 8. Using file: URLs internally allows packages to avoid dealing with platform-specific path representations. Please also note that this bug breaks all uses of the fs module with URLs, even when writing regular files (unrelated to dependencies).

@demurgos demurgos added the bug Something isn't working label Feb 5, 2020
@yarnbot

This comment has been minimized.

@yarnbot yarnbot added the unreproducible This issue cannot be reproduced on master label Feb 5, 2020
@demurgos demurgos changed the title [Bug] [Bug] URL instances are not supported by the FS layer Feb 5, 2020
@yarnbot

This comment has been minimized.

@yarnbot yarnbot added reproducible This issue can be successfully reproduced and removed unreproducible This issue cannot be reproduced on master labels Feb 5, 2020
@yarnbot
Copy link
Collaborator

yarnbot commented Feb 5, 2020

This issue reproduces on master:

Error: expect(received).not.toThrow()

Error name:    "TypeError"
Error message: "p.match is not a function"

      41827 | 
      41828 |   static resolveVirtual(p) {
    > 41829 |     const match = p.match(VIRTUAL_REGEXP);
            |                     ^
      41830 |     if (!match) return p;
      41831 |     const target = path_1.ppath.dirname(match[1]);
      41832 |     if (!match[3] || !match[4]) return target;

      at Function.resolveVirtual (../../github/workspace/.pnp.js:41829:21)
      at VirtualFS.mapToBase (../../github/workspace/.pnp.js:41866:22)
      at VirtualFS.fsMapToBase (../../github/workspace/.pnp.js:36849:19)
      at VirtualFS.writeFileSync (../../github/workspace/.pnp.js:36755:43)
      at PosixFS.writeFileSync (../../github/workspace/.pnp.js:36755:24)
      at evalmachine.<anonymous>:7:14
      at Object.<anonymous> (../../github/workspace/.yarn/cache/expect-npm-24.8.0-8c7640c562-2.zip/node_modules/expect/build/toThrowMatchers.js:81:11)
      at Object.throwingMatcher [as toThrow] (../../github/workspace/.yarn/cache/expect-npm-24.8.0-8c7640c562-2.zip/node_modules/expect/build/index.js:342:33)
      at module.exports (evalmachine.<anonymous>:7:58)
      at ../../github/workspace/.yarn/cache/@arcanis-sherlock-npm-1.0.38-d4f5e2dbf3-2.zip/node_modules/@arcanis/sherlock/lib/executeRepro.js:56:19
    at module.exports (evalmachine.<anonymous>:7:58)
    at /github/workspace/.yarn/cache/@arcanis-sherlock-npm-1.0.38-d4f5e2dbf3-2.zip/node_modules/@arcanis/sherlock/lib/executeRepro.js:56:19
    at executeInTempDirectory (/github/workspace/.yarn/cache/@arcanis-sherlock-npm-1.0.38-d4f5e2dbf3-2.zip/node_modules/@arcanis/sherlock/lib/executeRepro.js:17:22)
    at Object.executeRepro (/github/workspace/.yarn/cache/@arcanis-sherlock-npm-1.0.38-d4f5e2dbf3-2.zip/node_modules/@arcanis/sherlock/lib/executeRepro.js:24:18)
    at ExecCommand.execute (/github/workspace/.yarn/cache/@arcanis-sherlock-npm-1.0.38-d4f5e2dbf3-2.zip/node_modules/@arcanis/sherlock/lib/commands/exec.js:25:59)
    at async ExecCommand.validateAndExecute (/github/workspace/.yarn/cache/clipanion-npm-2.0.0-rc.16-b9444aaf89-2.zip/node_modules/clipanion/lib/advanced/Command.js:161:26)
    at async Cli.run (/github/workspace/.yarn/cache/clipanion-npm-2.0.0-rc.16-b9444aaf89-2.zip/node_modules/clipanion/lib/advanced/Cli.js:74:24)
    at async Cli.runExit (/github/workspace/.yarn/cache/clipanion-npm-2.0.0-rc.16-b9444aaf89-2.zip/node_modules/clipanion/lib/advanced/Cli.js:83:28)

demurgos added a commit to demurgos/turbo-gulp that referenced this issue Feb 5, 2020
The virtual FS used by Yarn does not support URL instances. This commit provides a temporary workaround by normalizing all paths to strings before calling into the fs module.
demurgos added a commit to demurgos/turbo-gulp that referenced this issue Feb 5, 2020
- **[Breaking change]** Drop support for `customTypingsDir` dir.
- **[Fix]** Update dependencies.
- **[Fix]** Add workaround for yarnpkg/berry#899.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working reproducible This issue can be successfully reproduced
Projects
None yet
2 participants