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

Support TypeScript declarations with @nx/esbuild:esbuild #20688

Closed
castleadmin opened this issue Dec 11, 2023 · 10 comments
Closed

Support TypeScript declarations with @nx/esbuild:esbuild #20688

castleadmin opened this issue Dec 11, 2023 · 10 comments
Assignees
Labels
outdated scope: bundlers Issues related to webpack, rollup type: feature

Comments

@castleadmin
Copy link
Contributor

Description

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.

Motivation

Declaration files (*.d.ts) are essential for published libraries.

Suggested Implementation

The commonly accepted workaround is to run a tsc build before the esbuild build that only outputs the declaration files.
This additional build step could be provided as a @nx/esbuild:esbuild option.

Alternate Implementations

Implement the additional build step in the @nx/js:library generator for publishable libraries.

@jaysoo
Copy link
Member

jaysoo commented Dec 11, 2023

@castleadmin Hey, thanks for expressing interest on this, do you have any suggestions on how you'd want to do this?

@castleadmin
Copy link
Contributor Author

Hi @jaysoo, for the suggested case I would do the following

  • Introduce a new boolean valued --declaration option
  • If --declaration or the tsconfig option declaration is true,
    then the TypeScript compiler will be used before the esbuild build call to generate the declarations
  • The TypeScript compiler could be called via the command line or the @nx/js:tsc executor
  • The same tsconfig options should be used for the esbuild and declarations build with one exception

@castleadmin
Copy link
Contributor Author

@jaysoo What do you think about the new feature? Should I start implementing a concrete solution?

@AgentEnder AgentEnder added the scope: bundlers Issues related to webpack, rollup label Dec 21, 2023
@GideonMax
Copy link

When can we expect this feature?

@castleadmin
Copy link
Contributor Author

I should be able to implement the feature within the next 2 weeks.

@ChristopherPHolder
Copy link

What is missing to complete this feature? It seems the MR is open and ready for review.

I am happy to contribute if there is anything needed to complete the feature 👍

@ThePlenkov
Copy link
Contributor

ThePlenkov commented Apr 4, 2024

Here is the way I have fixed it:


    "dts": {
      "executor": "@nx/js:tsc",
      "outputs": [
        "{options.outputPath}"
      ],
      "options": {
        "outputPath": "dist/packages/prosemirror-markdown-es",
        "main": "packages/prosemirror-markdown-es/src/index.ts",
        "tsConfig": "packages/prosemirror-markdown-es/tsconfig.dts.json",
        "clean": true
      }
    },
    "build": {
      "executor": "@nx/esbuild:esbuild",
      "outputs": [
        "{options.outputPath}"
      ],
      "options": {
        "outputPath": "dist/packages/prosemirror-markdown-es",
        "main": "packages/prosemirror-markdown-es/src/index.ts",
        "tsConfig": "packages/prosemirror-markdown-es/tsconfig.lib.json",
        "assets": [
          "packages/prosemirror-markdown-es/*.md",
        ],
        "generatePackageJson": true,
        "format": [
          "cjs"
        ],
        "deleteOutputPath": false
      },
      "dependsOn": [
        "dts"
      ]
    },

so I create first dts with clean:true and then build esbuild with "deleteOutputPath": false. I know it's ugly but it works =)

Addititionally I had to create a dedicated tsconfig too to use: "declaration": true and "emitDeclarationOnly": true,

mandarini pushed a commit that referenced this issue May 16, 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.
FrozenPandaz pushed a commit that referenced this issue 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)
@danielsharvey
Copy link
Contributor

This has been done in #21084. I have experienced and issue with referencing buildable libraries - see #26376.

@MaxKless
Copy link
Collaborator

Hello! Looks like this has been resolved in #21084 so I will close this issue.
Please create new issues or comment on the one linked above if you're running into problems with this feature.

Copy link

This issue has been closed for more than 30 days. If this issue is still occuring, please open a new issue with more recent context.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 27, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated scope: bundlers Issues related to webpack, rollup type: feature
Projects
None yet
Development

No branches or pull requests

10 participants