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

Current TypeScript packages setup and improvement suggestion #2051

Open
tash-2s opened this issue Dec 9, 2023 · 5 comments
Open

Current TypeScript packages setup and improvement suggestion #2051

tash-2s opened this issue Dec 9, 2023 · 5 comments

Comments

@tash-2s
Copy link
Contributor

tash-2s commented Dec 9, 2023

While working on my previous PR, I delved into the TypeScript packages setup in this repository. Here's my understanding, along with some issues I've found and a suggestion for improvement.

I'm not fully aware of the reasoning behind the current setup, so please correct me if I'm wrong or if I'm missing something.

Current package setup

  • Pure ESM package.
  • exports field points to compiled JS (.js) files.
  • types or typesVersions field points to source TS (.ts) files.
  • Does not generate DTS (TS type declaration, .d.ts) files.
  • moduleResolution in tsconfig.json is set to node, an alias of node10.

This setup seems designed to provide a good developer experience within this repository. Developers/tsc directly refer to the TS files of dependent packages, while Node.js or bundlers use the JS files.

Issues with this setup for MUD packages users

  1. Users need to include the @types/* dependencies, which are MUD packages' dev dependencies, in their projects to pass their tsc, even if they do not directly use these dependencies. This is because the packages expose TS files instead of DTS files, leading to projects' tsc treating them as regular in-project TS files. This setup may also increase the tsc time of the projects.
  2. Users must use specific configs in their tsconfig.json, particularly moduleResolution: node10. The recommended modern settings, bundler or node16, are not usable because these settings rely on exports fields rather than type / typesVersions fields for finding types, leading to errors due to missing type files.

Suggestion for improvement

The goal is to:

  • Maintain the current developer experience in this repository.
  • Avoid affecting existing (moduleResolution: node10) users.
  • Resolve the @types/* dependencies issue.
  • Allow users to opt for bundler or node16 in their projects.

Steps to improve:

  1. Generate DTS files (set dts: true in tsup.config.ts)
  2. Remove the @types/* necessity for node10 users (Optional)
    • This can be achieved by overriding types/typesVersions fields to point to DTS instead of TS on packing, using pnpm's publishConfig feature.
    • No impact on developers since the only change is the addition of publishConfig fields in packages' package.json.
  3. Switch to bundler or node16 from node10 in this repository's packages if desired (Optional)
    • The packages' package.json should have exports fields pointing to TS files and publishConfig.exports fields pointing to JS files.

Note

  • These changes have not been tested yet. I wanted to confirm my understanding of the current setup and know the desired future direction. (It seems like step 1 is working without major issues in my local /templates project with bundler and node16.)
  • Even when using MUD externally with a JS + DTS setting, I believe it's still possible to maintain the "go to TS definition" experience for users development environments by utilizing tsc --declarationMap.
@holic
Copy link
Member

holic commented Jan 2, 2024

I wanted to confirm my understanding of the current setup and know the desired future direction.

I don't think there was a ton of thought given to the current set up and likely is a result of copy+paste and iterative evolution. Maximum compatibility would be great with a preference towards user experience both for us building inside MUD monorepo and for users consuming the packages.

Even when using MUD externally with a JS + DTS setting, I believe it's still possible to maintain the "go to TS definition" experience for users development environments by utilizing tsc --declarationMap.

Would love for us to figure this out. My biggest beef with libraries using .d.ts is that they go to the type definition rather than the implementation. Often times when I am debugging or asking myself "how does this work internally", I want to see both!

@tash-2s
Copy link
Contributor Author

tash-2s commented Jan 2, 2024

Step 1 has been completed in #2084.

Please note that this PR does not affect users with moduleResolution: node10 (existing users and local MUD developers). MUD users with node10 will still need to install libraries like @types/prettier, as .ts files continue to be used instead of .d.ts files. Their tsc uses types/typesVersions rather than exports, and does not use the built .js and .d.ts files.

@tash-2s
Copy link
Contributor Author

tash-2s commented Jan 2, 2024

Often times when I am debugging or asking myself "how does this work internally", I want to see both!

I agree! Most libraries use a JS + DTS setup, but some libraries like viem include type declaration maps (.d.ts.map files generated by tsc --declarationMap) alongside DTS files. This allows users to directly navigate to TS files instead of DTS files.

@tash-2s
Copy link
Contributor Author

tash-2s commented Jan 2, 2024

As a follow-up to step 1, we should have CI to ensure that the DTS setup works in user environments.

@tash-2s
Copy link
Contributor Author

tash-2s commented Jan 4, 2024

Reflecting on #2036 (comment), here are the remaining tasks:

  • Include .d.ts.map files in MUD packages.
    • This isn't as simple as toggling a tsup config flag. Adjustments in the packages' tsconfig.json may be required.
  • Enable node10 users to use .d.ts files (step 2).
  • Switch to using bundler in MUD packages (step 3).
  • Ensure CI covers all supported TypeScript configurations for users.
  • Switch to using bundler in the templates.
  • clean up temp fix in fix(templates): fix templates tsc errors and add test #2036

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants