Skip to content

Comments

build(linter/plugins): simplify TSDown config#15948

Merged
graphite-app[bot] merged 1 commit intomainfrom
11-21-build_linter_plugins_simplify_tsdown_config
Nov 21, 2025
Merged

build(linter/plugins): simplify TSDown config#15948
graphite-app[bot] merged 1 commit intomainfrom
11-21-build_linter_plugins_simplify_tsdown_config

Conversation

@overlookmotel
Copy link
Member

@overlookmotel overlookmotel commented Nov 21, 2025

#15946 means we can now simplify the TSDown build config.

Previously we had to have 2 separate builds for index.ts and cli.ts, to avoid assert* functions ending up in a shared chunk, and not getting removed by minifier. Now that problem is solved, so we can switch to a single build with 2 entry points.

TSDown does create a small extra chunk for its __toESM and __commonJSMin functions, but I think that's fine.

Annoyingly it doesn't seem to be possible to tell TSDown to generate .d.ts files only for the index.js chunk, so have to delete this pointless files in build script.

The point of all this is that RuleTester needs to use a ton of code from plugins.ts, and we wouldn't want all that code duplicated in both index and plugins chunks (especially once we're bundling TS-ESlint's parser #15861). Now that code will be shared between the two entry points in a shared chunk, not duplicated.

Copy link
Member Author

overlookmotel commented Nov 21, 2025


How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • 0-merge - adds this PR to the back of the merge queue
  • hotfix - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@github-actions github-actions bot added A-linter Area - Linter A-cli Area - CLI labels Nov 21, 2025
Copy link
Member Author

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more


How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • 0-merge - adds this PR to the back of the merge queue
  • hotfix - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@overlookmotel overlookmotel force-pushed the 11-21-refactor_linter_plugins_replace_with_assertisnonnull_ branch from 5d340df to ba7b9b6 Compare November 21, 2025 14:31
@overlookmotel overlookmotel force-pushed the 11-21-build_linter_plugins_simplify_tsdown_config branch from 75bcbdb to 5633be4 Compare November 21, 2025 14:31
@overlookmotel overlookmotel added the A-linter-plugins Area - Linter JS plugins label Nov 21, 2025
@graphite-app graphite-app bot force-pushed the 11-21-refactor_linter_plugins_replace_with_assertisnonnull_ branch from ba7b9b6 to dacaaf2 Compare November 21, 2025 14:50
@graphite-app graphite-app bot force-pushed the 11-21-build_linter_plugins_simplify_tsdown_config branch from 5633be4 to f1a8cbc Compare November 21, 2025 14:50
@graphite-app graphite-app bot changed the base branch from 11-21-refactor_linter_plugins_replace_with_assertisnonnull_ to graphite-base/15948 November 21, 2025 14:57
@graphite-app graphite-app bot force-pushed the 11-21-build_linter_plugins_simplify_tsdown_config branch from f1a8cbc to cda76ed Compare November 21, 2025 15:05
@graphite-app graphite-app bot force-pushed the graphite-base/15948 branch from dacaaf2 to 152e5de Compare November 21, 2025 15:05
@graphite-app graphite-app bot changed the base branch from graphite-base/15948 to main November 21, 2025 15:06
@graphite-app graphite-app bot force-pushed the 11-21-build_linter_plugins_simplify_tsdown_config branch from cda76ed to ad43f29 Compare November 21, 2025 15:06
@graphite-app graphite-app bot changed the base branch from main to graphite-base/15948 November 21, 2025 15:42
@graphite-app graphite-app bot force-pushed the 11-21-build_linter_plugins_simplify_tsdown_config branch from ad43f29 to 2a9ead3 Compare November 21, 2025 15:48
@graphite-app graphite-app bot changed the base branch from graphite-base/15948 to main November 21, 2025 15:49
@graphite-app graphite-app bot force-pushed the 11-21-build_linter_plugins_simplify_tsdown_config branch from 2a9ead3 to 72da02e Compare November 21, 2025 15:49
Copilot AI review requested due to automatic review settings November 21, 2025 16:04
@overlookmotel overlookmotel force-pushed the 11-21-build_linter_plugins_simplify_tsdown_config branch from 72da02e to d6d14a3 Compare November 21, 2025 16:04
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR simplifies the TSDown build configuration for the oxlint package by consolidating two separate build configurations into a single build with multiple entry points. This change is made possible by a previous fix (#15946) that resolved issues with assert* functions ending up in shared chunks.

Key changes:

  • Merged separate cli.ts and index.ts builds into a single config with two entry points
  • Added post-build cleanup to remove unwanted cli.d.ts declaration files
  • Updated debug config generation to use object spread from the main config

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
apps/oxlint/tsdown.config.ts Consolidated two separate TSDown configs into one with multiple entry points; simplified debug config creation
apps/oxlint/scripts/build.ts Added rmSync import and logic to delete generated cli.d.ts files from both dist and debug directories

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@overlookmotel overlookmotel force-pushed the 11-21-build_linter_plugins_simplify_tsdown_config branch from d6d14a3 to 9c002a1 Compare November 21, 2025 16:07
@overlookmotel overlookmotel added the 0-merge Merge with Graphite Merge Queue label Nov 21, 2025
Copy link
Member Author

overlookmotel commented Nov 21, 2025

Merge activity

#15946 means we can now simplify the TSDown build config.

Previously we had to have 2 separate builds for `index.ts` and `cli.ts`, to avoid `assert*` functions ending up in a shared chunk, and not getting removed by minifier. Now that problem is solved, so we can switch to a single build with 2 entry points.

TSDown does create a small extra chunk for its `__toESM` and `__commonJSMin` functions, but I think that's fine.

Annoyingly it doesn't seem to be possible to tell TSDown to generate `.d.ts` files only for the `index.js` chunk, so have to delete this pointless files in build script.

The point of all this is that `RuleTester` needs to use a ton of code from `plugins.ts`, and we wouldn't want all that code duplicated in both `index` and `plugins` chunks (especially once we're bundling TS-ESlint's parser #15861). Now that code will be shared between the two entry points in a shared chunk, not duplicated.
@graphite-app graphite-app bot force-pushed the 11-21-build_linter_plugins_simplify_tsdown_config branch from 9c002a1 to 8b3fd1c Compare November 21, 2025 16:18
@graphite-app graphite-app bot merged commit 8b3fd1c into main Nov 21, 2025
18 checks passed
@graphite-app graphite-app bot deleted the 11-21-build_linter_plugins_simplify_tsdown_config branch November 21, 2025 16:23
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Nov 21, 2025
taearls pushed a commit to taearls/oxc that referenced this pull request Dec 11, 2025
oxc-project#15946 means we can now simplify the TSDown build config.

Previously we had to have 2 separate builds for `index.ts` and `cli.ts`, to avoid `assert*` functions ending up in a shared chunk, and not getting removed by minifier. Now that problem is solved, so we can switch to a single build with 2 entry points.

TSDown does create a small extra chunk for its `__toESM` and `__commonJSMin` functions, but I think that's fine.

Annoyingly it doesn't seem to be possible to tell TSDown to generate `.d.ts` files only for the `index.js` chunk, so have to delete this pointless files in build script.

The point of all this is that `RuleTester` needs to use a ton of code from `plugins.ts`, and we wouldn't want all that code duplicated in both `index` and `plugins` chunks (especially once we're bundling TS-ESlint's parser oxc-project#15861). Now that code will be shared between the two entry points in a shared chunk, not duplicated.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-cli Area - CLI A-linter Area - Linter A-linter-plugins Area - Linter JS plugins

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants