yarnpkg / berry

πŸ“¦πŸˆ Active development trunk for Yarn βš’
https://yarnpkg.com
BSD 2-Clause "Simplified" License
7.45k stars 1.11k forks source link

[Bug?]: TypeScript project references do not work with virtual packages #3829

Open swandir opened 2 years ago

swandir commented 2 years ago

Self-service

Describe the bug

TypeScript project references fail to work properly with virtual package paths.

When using project references, module imports should be replaced by TypeScript with corresponding .d.ts files, which fails when a referenced workspace is virtualized, resulting in leaking of imported module source code into compilation.

To reproduce

const fs = require("fs/promises");

await fs.mkdir("workspaces");
await fs.mkdir("workspaces/a");
await fs.mkdir("workspaces/b");
await fs.mkdir("workspaces/c");

// Root

await fs.writeFile("package.json", `\
{
  "workspaces": [
    "workspaces/*"
  ],
  "devDependencies": {
    "typescript": "^4.5.2"
  }
}
`);

// Workspace A

await fs.writeFile("workspaces/a/package.json", `\
{
  "name": "a",
  "exports": "./index.ts",
  "types": "index.ts",
  "dependencies": {
    "b": "workspace:*",
    "c": "workspace:*"
  }
}
`);

await fs.writeFile("workspaces/a/tsconfig.json", `\
{
  "include": ["index.ts"],
  "references": [{ "path": "../b" }, { "path": "../c" }],
  "compilerOptions": {
    "composite": true,
    "declaration": true,
    "emitDeclarationOnly": true,
    "skipLibCheck": true,
    "moduleResolution": "node",
    "module": "esnext",
    "strict": true
  }
}
`);

await fs.writeFile("workspaces/a/index.ts", `\
import { b } from "b";

b();
`);

// Workspace B

await fs.writeFile("workspaces/b/package.json", `\
{
  "name": "b",
  "exports": "./index.ts",
  "types": "index.ts",
  "peerDependencies": {
    "c": "*"
  },
  "devDependencies": {
    "c": "workspace:*"
  }
}
`);

await fs.writeFile("workspaces/b/tsconfig.json", `\
{
  "include": ["index.ts"],
  "references": [{ "path": "../c" }],
  "compilerOptions": {
    "composite": true,
    "declaration": true,
    "emitDeclarationOnly": true,
    "skipLibCheck": true,
    "moduleResolution": "node",
    "module": "esnext"
  }
}
`);

await fs.writeFile("workspaces/b/index.ts", `\
import { c } from "c";

c();

export const b = (...args) => args;
`);

// Workspace C

await fs.writeFile("workspaces/c/package.json", `\
{
  "name": "c",
  "exports": "./index.ts",
  "types": "index.ts"
}
`);

await fs.writeFile("workspaces/c/tsconfig.json", `\
{
  "include": ["index.ts"],
  "compilerOptions": {
    "composite": true,
    "declaration": true,
    "emitDeclarationOnly": true,
    "skipLibCheck": true,
    "moduleResolution": "node",
    "module": "esnext"
  }
}
`);

await fs.writeFile("workspaces/c/index.ts", `\
import "b";
export const c = (...args) => args;
`);

//

await global.yarn("install");

let tscError
try {
  await global.yarn("tsc",  "-b", "workspaces/a");
} catch (e) {
  tscError = e.message;
}

expect(tscError).toBe(undefined);

Here's a repo with three workspaces, one of which gets virtualized. This setup works with PnP disabled, but fails with it enabled.

https://github.com/swandir/yarn-virtual-project-references

Environment

System:
  OS: macOS 12.0.1
  CPU: (12) x64 Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
Binaries:
  Node: 17.0.1 - /private/var/folders/4z/nlhtwc4s6psgn6y7t6zx4dpwlypf3b/T/xfs-49bdfe6d/node
  Yarn: 3.1.1 - /private/var/folders/4z/nlhtwc4s6psgn6y7t6zx4dpwlypf3b/T/xfs-49bdfe6d/yarn
  npm: 8.1.0 - /usr/local/bin/npm

Additional context

When you build a composite project using build mode and project references, during compilation TypeScript replaces real modules of a referenced projects with corresponding d.ts modules.

As an effect, TypeScript doesn't check full source code of a referenced project, which allows, for example, to have different options for referencer and referencee packages (same as with bundled packages).

The problem is that it fails to work properly with virtual packages.

As long as workspaces are not virtualized, paths are resolved to real fs paths, and TypeScript is able to match them to corresponding d.ts.

With virtual packages seems like path matching fails, and TypeScript uses real modules.

It may fail silently when TypeScript options are compatible across workspaces, but if, for example, a referencer project has strict enabled whereas the referencee does not, compilation of the referencer will fail with an error inside the referencee.

swandir commented 2 years ago

As a workaround this works: https://github.com/yarnpkg/berry/issues/1932#issuecomment-704142945

swandir commented 2 years ago

On a different note, do workspaces really need to be virtualized in this case?

swandir commented 2 years ago

As a workaround this works: #1932 (comment)

Though it only works as long as all dependent-on workspaces are listed before their dependants.

Let's say we have workspaces A, B and C. A has B and C in its dependencies. B has C in its peerDependencies.

This tsconfig for A works:

{
  "references": [
    { "path": "../C" },
    { "path": "../B" },
  ]
}

And this does not:

{
  "references": [
    { "path": "../B" },
    { "path": "../C" },
  ]
}
swandir commented 2 years ago

Though it only works as long as all dependent-on workspaces are listed before their dependants.

Apparently because, when given references in that order, tsc skips compilation of a transitive workspace since it's already built.

yarnbot commented 2 years ago

Hi! πŸ‘‹

This issue looks stale, and doesn't feature the reproducible label - which implies that you didn't provide a working reproduction using Sherlock. As a result, it'll be closed in a few days unless a maintainer explicitly vouches for it or you edit your first post to include a formal reproduction (you can use the playground for that).

Note that we require Sherlock reproductions for long-lived issues (rather than standalone git repositories or similar) because we're a small team. Sherlock gives us the ability to check which bugs are still affecting the master branch at any given point, and decreases the amount of code we need to run on our own machines (thus leading to faster bug resolutions). It helps us help you! πŸ˜ƒ

If you absolutely cannot reproduce a bug on Sherlock (for example because it's a Windows-only issue), a maintainer will have to manually add the upholded label. Thanks for helping us triaging our repository! 🌟

swandir commented 2 years ago

Added a Sherlock repro if it helps, though it's a bit long πŸ˜…

yarnbot commented 2 years ago

This issue reproduces on master:

Error: expect(received).toBe(expected) // Object.is equality

Expected: undefined
Received: "Command failed: /usr/bin/node /github/workspace/scripts/actions/../run-yarn.js tsc -b workspaces/a

.yarn/__virtual__/b-virtual-0b8cdd16e3/1/workspaces/b/index.ts(5,19): error TS7019: Rest parameter 'args' implicitly has an 'any[]' type.
"
    at module.exports (evalmachine.<anonymous>:137:18)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at async /github/workspace/.yarn/cache/@arcanis-sherlock-npm-2.0.3-558f52b79f-ccfa417b92.zip/node_modules/@arcanis/sherlock/lib/executeRepro.js:57:13
    at async executeInTempDirectory (/github/workspace/.yarn/cache/@arcanis-sherlock-npm-2.0.3-558f52b79f-ccfa417b92.zip/node_modules/@arcanis/sherlock/lib/executeRepro.js:18:16)
    at async executeRepro (/github/workspace/.yarn/cache/@arcanis-sherlock-npm-2.0.3-558f52b79f-ccfa417b92.zip/node_modules/@arcanis/sherlock/lib/executeRepro.js:25:12)
    at async ExecCommand.execute (/github/workspace/.yarn/cache/@arcanis-sherlock-npm-2.0.3-558f52b79f-ccfa417b92.zip/node_modules/@arcanis/sherlock/lib/commands/exec.js:26:38)
    at async ExecCommand.validateAndExecute (/github/workspace/.yarn/cache/clipanion-npm-2.0.0-rc.16-b9444aaf89-91cf93ba72.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-91cf93ba72.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-91cf93ba72.zip/node_modules/clipanion/lib/advanced/Cli.js:83:28)
swandir commented 2 years ago

What would be the preferred way to tackle this problem?

(1) Seems like the most straightforward and transparent to the user is to avoid resolving workspaces to virtual paths. At first thought it seems fine since a package resolved to a workspace will have the same version and contents regardless of the path through which it was resolved. Would it work?

(2) An alternative would be to make TypeScript take into account virtual paths while it does the ts to d.ts substitution for referenced project. But this change sounds intrusive and I think virtual paths aren't a part of PnP spec?

Otherwise, PnP appears to break the expectation of workspace dependencies being resolved to a known source code location. An expectation that references rely on.

(3) That expectation might be wrong, in which case it probably should be documented along with a workaround for this kind of use-cases. Though I'm not sure what a reasonable workaround could be, given that virtual paths may change unexpectedly.

swandir commented 2 years ago

(2) An alternative would be to make TypeScript take into account virtual paths while it does the ts to d.ts substitution for referenced project. But this change sounds intrusive and I think virtual paths aren't a part of PnP spec?

It would work without relying on PnP directly if TypeScript resolved project references as modules. https://github.com/microsoft/TypeScript/issues/44034

adrian-gierakowski commented 11 months ago

I wonder if this is still an issue? Could the repro be edited and arn against latest version of typescript and yarn?

swandir commented 11 months ago

Yeah, still an issue.

Sherlock repros don't seem to work anymore, so I've updated the external repro

https://github.com/swandir/yarn-virtual-project-references