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

tsup's code splitting feature makes core packages difficult to debug #4333

Closed
mcmire opened this issue May 28, 2024 · 1 comment · Fixed by #4648
Closed

tsup's code splitting feature makes core packages difficult to debug #4333

mcmire opened this issue May 28, 2024 · 1 comment · Fixed by #4648

Comments

@mcmire
Copy link
Contributor

mcmire commented May 28, 2024

Currently, when tsup transpiles code in CommonJS format, it assembles common code into "chunk" files. These files make life difficult for the extension team when they want to debug behavior in a core package, because they do not necessarily have a 1-to-1 relationship between existing files; they may map to multiple files in the original source code. They are also randomly named. This makes it impossible to know what they contain without opening them.

@mcmire mcmire self-assigned this May 28, 2024
@mcmire mcmire changed the title Research configuring tsup to not create chunk files tsup's code splitting feature makes core packages difficult to debug May 28, 2024
@mcmire
Copy link
Contributor Author

mcmire commented May 28, 2024

I did some investigation into how we might be able to solve this problem.

Ideally, the build process should work like this:

  • We still need to create two builds for each package, a CommonJS build and an ESM build. However, they should work differently.

    • The CommonJS build should create one .cjs file per export. Currently, no packages have any exports other than the default one, so all code should be bundled into an index.cjs (no code splitting). Type declaration files should be created per file, but each file should end in .d.cts and each import in that file should refer to a .d.cts file.
    • The ESM build should create 1 JavaScript file per TypeScript file in the original source, but each file should end in .mjs. Type declaration files should also be created per file as with CommonJS, but each file should end in .d.mts and each import in that file should refer to a .d.mts file.
  • The package.json for each package should minimally contain:

    "sideEffects": false,  
    "exports": {
      ".": {
        "import": {
          "types": "./dist/index.d.mts",
          "default": "./dist/index.mjs"
        },
        "require": {
          "types": "./dist/index.d.cts",
          "default": "./dist/index.cjs"
        }
      },
    },
    "main": "./dist/index.js",
    "types": "./dist/types/index.d.ts",
    

Ordinarily, we could fulfill this plan by moving to ts-bridge. However, this solution is complicated by the fact that we use TypeScript's project references feature, and ts-bridge does not support project references. There is an open ticket around this, but I am unsure how complicated it would be to fully resolve that problem.

One question that might come up is: do we really need to use project references?

  • Argument for: TypeScript is aware of the relationships between each package, so it can build them in the correct (topological) order.
  • Argument for: Project references make building the whole monorepo potentially faster as TypeScript does not need to compile do not need to be compiled again. However, we don't build the whole monorepo often; we only really do that in CI.
  • Argument for: VSCode makes use of project references and builds a "shadow representation" of your project in the background so that you can access types of any file in any other file easily. Removing project references may make VSCode slower when it comes to reading TypeScript files (investigation would be needed).
  • Argument against: The list of project references is essentially a repeat of the list of public workspaces that Yarn knows about (there is an open TypeScript issue about this in which people have posted various solutions for syncing the two)
  • Argument against: Redux Toolkit does not use project references. Looking at this repo, there is no top-level tsconfig.json; instead, each package has its own tsconfig.json, and it is disconnected from the others. There is a top-level build package script, but each package is iterated over in topological order, and then tsup is run for each project. That said, this repo is rather small, so perhaps the benefits of using project references do not outweight the inconveniences.
  • Argument for: Jest does use project references. It doesn't have a top-level solution file, but each package contains references to others, for example this one. They have custom scripts for compiling TypeScript, but here is where they run tsc --build.

So it seems like we have one of two paths we can take depending on our decision:

  • If we no longer use project references, we may be able to discard tsup, and we may be able to iterate through packages in topological order and run ts-bridge on each instead (provided that this tool still does what we want). However, we would need to investigate whether this affects the performance of VSCode.
  • If we continue to use project references, then I am unsure how to proceed, short of trying out the solutions posted in Support project references ts-bridge/ts-bridge#3.

@desi desi unassigned mcmire Jul 11, 2024
Mrtenz added a commit that referenced this issue Sep 16, 2024
## Explanation

`ts-bridge` finally supports project references. In this PR, I've
swapped out `tsup` for `ts-bridge` everywhere.

## References

Related to MetaMask/metamask-module-template#247, MetaMask/utils#182.
Closes #4333.

## Checklist

- [x] I've updated the test suite for new or updated code as appropriate
- [x] I've updated documentation (JSDoc, Markdown, etc.) for new or
updated code as appropriate
- [x] I've highlighted breaking changes using the "BREAKING" category
above as appropriate
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant