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

Why is main not set to dist/ in email-send-1 #1

Open
PaulRBerg opened this issue Jun 4, 2023 · 9 comments
Open

Why is main not set to dist/ in email-send-1 #1

PaulRBerg opened this issue Jun 4, 2023 · 9 comments

Comments

@PaulRBerg
Copy link

Based on what I understand from ChatGPT, main should point to the built JavaScript files, not to the TypeScript source code. Or am I (and ChatGPT) wrong?

@mxro
Copy link
Collaborator

mxro commented Jun 6, 2023

Thank you for raising this issue!

The reason why this is set to the TypeScript source as opposed to the compiled source is so that IntelliSense and the bundler always refer to the original TypeScript source. Providing more convenient look up (e.g. go to declaration will go to the source) and providing the bundler and other tooling access to the original source, which I think could potentially be used for better optimisation (e.g. I am still struggling to get tree shaking working off the compiled JS.

Since this package should never be used outside the monorepo, this should not cause any problems here. If the package should be published to npm and imported in other workspaces, Yarn allows overriding the main in the publishConfig to the JS file, such as done here: https://github.com/goldstack/goldstack/blob/master/workspaces/templates-lib/packages/template-email-send/package.json#L69. This should then be automatically applied when running yarn publish.

Getting everything in the workspace to resolve correctly for local development, bundling and publishing was a bit of a delicate matter initially but the current set up has worked quite well for a while. However, if there is any new developments that I may have missed providing a different alternative hear, I will be very happy to learn about it!

@PaulRBerg
Copy link
Author

PaulRBerg commented Jun 7, 2023

I thank you for the detailed answer!

The reason why this is set to the TypeScript source ... is so that IntelliSense and the bundler always refer to the original TypeScript source

I see.

Yarn allows overriding the main in the publishConfig to the JS file

Aah, I didn't know about this. Pretty cool.

if there is any new developments that I may have missed providing a different alternative hear

I think the standard way to achieving your goal (IDE integration) is to use the approach popularized by Andrei Picus in his article How to set up a TypeScript monorepo and make Go to definition work, which involves using multiple TSConfig files. An example can be found here:

https://github.com/NiGhTTraX/ts-monorepo

One file, tsconfig.json, is used for type checking and IDE integration, while the other, tsconfig.build.json, is used for building the packages. The former uses paths to tell the IDE where to find the source TypeScript code, while the latter doesn't use any paths and relies on a workspace management tool like Lerna to build all packages topologically (i.e., dependencies are built first). The advantage of this approach (compared to pointing to src in main) is that the build and publish scripts are more tightly coupled - dist is always assumed to be the main entrypoint.

Nonetheless, I agree that there is little difference between the two approaches. Turborepo recommends doing what you have done in their article You might not need TypeScript project references.

@mxro
Copy link
Collaborator

mxro commented Jun 9, 2023

Thank you for sending through the article. Yes, I think that could also work! However, I think I prefer the current approach in this repo due to:

  • No need to maintain a separate tsconfig file
  • No need to keep paths manually up to date

This monorepo already provides a tool to keep TypeScript references up to date. I think that alone is probably sufficient to have the go to declaration work: https://github.com/goldstack/goldstack/tree/master/workspaces/utils/packages/utils-typescript-references#readme

TypeScript is quite optimised to avoid rebuilding modules references through TypeScript references (and this monorepo is configured for this), so I don't think the approach from the Turborepo article is required.

Btw, in an earlier iteration of this monorepo it was configured to resolve all main directly to the JS source and use that as primary means to import packages (as opposed to TypeScript project references used now). However, this led to huge headaches, chief among them the need to build the packages on demand when they change. TypeScript project references takes care of that in the background and generally seems to work very well.

In any case, always happy to be convinced otherwise 😄

@PaulRBerg
Copy link
Author

Thanks for the additional color.

I am with you that Project References are the way to go for TypeScript-only monorepos, but they cannot be used with tools like Next.js, which use a different compiler (SWC). I've actually opened a discussion in the Next.js forum to ask for guidance on this situation: vercel/next.js#50866.

My current thinking is that it is not possible to use project references with Next.js, and the only way to make Next.js work in a monorepo is to have multiple TSConfig files (OR do what Turborepo recommends, if all of your packages are meant to remain private forever).

it was configured to resolve all main directly to the JS source and use that as primary means to import packages (as opposed to TypeScript project references used now).

Wait, this confuses me!

aren't project references supposed to make TypeScript build the dependencies in a topological way, and thus have the compiler use the built files from 'dist` (the JS source)?

@mxro
Copy link
Collaborator

mxro commented Jun 10, 2023

That's a good question, will follow it as well!

Note I have also a self-updating monorepo template specifically for Next.js related to this template here: https://github.com/goldstack/nextjs-bootstrap-boilerplate

This works with next-transpile-modules and a little extra script: https://github.com/goldstack/nextjs-bootstrap-boilerplate/blob/master/packages/app-nextjs-bootstrap-1/scripts/getLocalPackages.js

But generally seems to work without any issues!

@PaulRBerg
Copy link
Author

I have also a self-updating monorepo template

Ah, I didn't see that - thanks for sharing. I'll have a look.

This works with next-transpile-modules

FYI that package is no longer needed since Next.js v13.1

@mxro
Copy link
Collaborator

mxro commented Jun 11, 2023

That is great to know on next-transpile-modules 🥳 - I have removed this from the template now with goldstack/goldstack#322

@PaulRBerg
Copy link
Author

Sorry to bug you again, @mxro, but I think that you missed my question above? This:

aren't project references supposed to make TypeScript build the dependencies in a topological way, and thus have the compiler use the built files from 'dist` (the JS source)?

@mxro
Copy link
Collaborator

mxro commented Jun 11, 2023

Oh yes, sorry for missing the question!

I think generally the answer to this is yes! However, there are a few special circumstances to keep in mind:

e.g. see here

Pointing esbuild at the generated Javascript for your local dependency packages results in significantly higher bundle size than pointing esbuild at their raw Typescript sources:

evanw/esbuild#1250 (comment)

Thus I think generally it is best to hook up TypeScript files directly as opposed to going through the generate JS and d.ts files.

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