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

feat(bundling): added support for declarations (*.d.ts) #21084

Merged
merged 1 commit into from
May 16, 2024

Conversation

castleadmin
Copy link
Contributor

Current Behavior

esbuild doesn't support the creation of declaration files (*.d.ts) and probably never will (see evanw/esbuild#95).
Since declaration files are essential for published libraries,
it would be great if @nx/esbuild:esbuild would provide an option to output them.

Expected Behavior

  • Introduced a new boolean valued declaration option for the esbuild executor
  • If declaration or the tsconfig option declaration is true,
    then the TypeScript compiler is used before esbuild to generate the declarations
  • The output directory structure of the declarations can be influenced by setting the TypeScript rootDir via the declarationRootDir option

Related Issue(s)

#20688

Additional Comment

Please note that the generated declaration files directory structure is affected by the tsconfig rootDir property.

For a library that doesn't reference other libraries inside the monorepo, the rootDir property can be changed freely.
If a library inside the monorepo is referenced the rootDir property must be set to the workspace root.

The tsc executor has a sophisticated check that automatically sets the rootDir to the workspace root if a library is referenced.
https://github.com/nrwl/nx/blob/master/packages/js/src/executors/tsc/tsc.impl.ts#L104

This check is quite complex and specific to the tsc executor options. Therefore, it hasn't been included inside the esbuild implementation.

The current implementation leaves it to the user to solve the edge case by removing the declarationRootDir option or by setting the declarationRootDir to ..

In the future, it might make sense to generalize and use the tsc executor check.

Copy link

vercel bot commented Jan 10, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
nx-dev ✅ Ready (Inspect) Visit Preview May 9, 2024 2:08pm

@Afellman
Copy link

Any progress on this? My team is waiting for it.

@mandarini
Copy link
Member

Tested this locally, and the tests on CI seem to pass, so I am going to merge.

However, once esbuild is part of the crystal project, this functionality will go away, since we will be relying on the native features of esbuild.

@mandarini mandarini merged commit 7f32d86 into nrwl:master May 16, 2024
6 checks passed
@danielsharvey
Copy link
Contributor

Tested this locally, and the tests on CI seem to pass, so I am going to merge.

Thank you

However, once esbuild is part of the crystal project, this functionality will go away, since we will be relying on the native features of esbuild.

What approach would you recommend in the context of Project Crystal?

@castleadmin
Copy link
Contributor Author

An esbuild plugin could be a viable alternative to support declaration file builds.

esbuild-plugin-d.ts is listed on the esbuild plugin page and provides the needed functionality.

Another solution that doesn't rely on third-party plugins would be to split the current implementation into 2 commands.

  • Build the declaration files with the help of tsc
  • Build the project with esbuild

If the clearing of the output directory and caching doesn't interfere, then a solution that uses 2 commands should work fine.

@danielsharvey
Copy link
Contributor

danielsharvey commented May 17, 2024

Another solution that doesn't rely on third-party plugins would be to split the current implementation into 2 commands.

  • Build the declaration files with the help of tsc
  • Build the project with esbuild

👍🏻 This is what I currently do (though an OOB integrated solution would be nice!)

FrozenPandaz pushed a commit that referenced this pull request May 21, 2024
<!-- Please make sure you have read the submission guidelines before
posting an PR -->
<!--
https://github.com/nrwl/nx/blob/master/CONTRIBUTING.md#-submitting-a-pr
-->

<!-- Please make sure that your commit message follows our format -->
<!-- Example: `fix(nx): must begin with lowercase` -->

## Current Behavior
esbuild doesn't support the creation of declaration files (*.d.ts) and
probably never will (see evanw/esbuild#95).
Since declaration files are essential for published libraries,
it would be great if @nx/esbuild:esbuild would provide an option to
output them.

## Expected Behavior

- Introduced a new boolean valued `declaration` option for the `esbuild`
executor
- If `declaration` or the tsconfig option
[declaration](https://www.typescriptlang.org/tsconfig#declaration) is
true,
then the TypeScript compiler is used before esbuild to generate the
declarations
- The output directory structure of the declarations can be influenced
by setting the TypeScript rootDir via the `declarationRootDir` option

## Related Issue(s)
#20688

### Additional Comment
Please note that the generated declaration files directory structure is
affected by the tsconfig `rootDir` property.

For a library that doesn't reference other libraries inside the
monorepo, the `rootDir` property can be changed freely.
If a library inside the monorepo is referenced the `rootDir` property
must be set to the workspace root.

The `tsc` executor has a sophisticated check that automatically sets the
`rootDir` to the workspace root if a library is referenced.

https://github.com/nrwl/nx/blob/master/packages/js/src/executors/tsc/tsc.impl.ts#L104

This check is quite complex and specific to the `tsc` executor options.
Therefore, it hasn't been included inside the esbuild implementation.

The current implementation leaves it to the user to solve the edge case
by removing the `declarationRootDir` option or by setting the
`declarationRootDir` to `.`.

In the future, it might make sense to generalize and use the `tsc`
executor check.

(cherry picked from commit 7f32d86)
Copy link

This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants