Skip to content

chore: trim runtime dependencies — remove enhanced-resolve/semver, swap micromatch → picomatch#1701

Open
Copilot wants to merge 11 commits into
mainfrom
copilot/analyze-ts-loader-dependencies
Open

chore: trim runtime dependencies — remove enhanced-resolve/semver, swap micromatch → picomatch#1701
Copilot wants to merge 11 commits into
mainfrom
copilot/analyze-ts-loader-dependencies

Conversation

Copilot AI commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Three runtime dependencies were either dead weight or heavier than necessary. This PR removes them or substitutes lighter alternatives.

Changes

  • enhanced-resolve removed — the import in resolver.ts was already unreachable; the resolver body unconditionally throws and has been disabled since v9.4.1
  • semver removed — sole usage was a single gte(version, '3.6.3') call; the version check has been dropped entirely since TypeScript versions below 3.6.3 are no longer a concern
  • micromatchpicomatch — micromatch itself wraps picomatch; the one call-site in utils.ts maps directly, with negation patterns handled by splitting positive and negative entries and applying them separately to preserve identical glob/negation semantics

devDependencies

@types/semver and @types/micromatch removed; @types/picomatch added.

Copilot AI changed the title chore: remove enhanced-resolve and semver; replace micromatch with picomatch chore: trim runtime dependencies — remove enhanced-resolve/semver, swap micromatch → picomatch Jun 19, 2026
Copilot AI requested a review from johnnyreilly June 19, 2026 14:30
@johnnyreilly

Copy link
Copy Markdown
Member

@copilot The comparison tests are reliably failing now - please determine the cause

Copilot AI commented Jun 19, 2026

Copy link
Copy Markdown
Contributor Author

@copilot The comparison tests are reliably failing now - please determine the cause

Found and fixed. The reportFiles test was failing because picomatch handles negation patterns differently from micromatch. When passed an array like ['**/*.ts', '!skip.ts'], picomatch treats elements as a union (OR), so !skip.ts means "anything that isn't skip.ts" — meaning skip.ts still matched via **/*.ts. micromatch instead interprets negation entries as exclusions from the positive match set.

The fix splits the pattern array into positive and negative groups and applies them separately:

const matchPos = picomatch(positivePatterns.length > 0 ? positivePatterns : ['**']);
const matchNeg = negativePatterns.length > 0 ? picomatch(negativePatterns) : null;
if (!matchPos(relativeFileName) || (matchNeg && matchNeg(relativeFileName))) {
  return false;
}

Fix committed in the latest push. The reportFiles test now passes locally.

Comment thread src/compilerSetup.ts Outdated
if (
compiler!.version !== undefined &&
semver.gte(compiler!.version, '3.6.3')
versionGte(compiler!.version, '3.6.3')

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@copilot please remove versionGte(compiler!.version, '3.6.3') test from the if statement

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done. Removed versionGte(compiler!.version, '3.6.3') from the condition — the if now simply checks compiler!.version !== undefined. The versionGte helper function was also removed since it had no remaining usages.

Comment thread src/utils.ts Outdated
loaderOptions.reportFiles
// Split positive and negative patterns to replicate micromatch semantics:
// a file must match a positive pattern AND not match any negative pattern.
const positivePatterns = loaderOptions.reportFiles.filter(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@copilot please replace positivePatterns / negativePatterns creation with one reduce statement instead of 2 filters

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done — replaced the two filter calls with a single reduce that partitions patterns into positivePatterns and negativePatterns in one pass.

Copilot AI commented Jun 19, 2026

Copy link
Copy Markdown
Contributor Author

@copilot The comparison tests are reliably failing now - please determine the cause

The CI failures were caused by the reportFiles test. The initial picomatch swap broke negation-pattern semantics: when passed ['**/*.ts', '!skip.ts'], picomatch treats each element as a union (OR), so !skip.ts matched everything except skip.ts but **/*.ts still caught it. The fix (commit 532b0de) splits patterns into positive and negative groups and applies them separately — a file must match a positive pattern AND not match any negative pattern. The subsequent CI runs passed after that fix landed.

@johnnyreilly

Copy link
Copy Markdown
Member

@copilot increment the version in package.json and add an entry to the changelog.md

Copilot AI commented Jun 19, 2026

Copy link
Copy Markdown
Contributor Author

@copilot increment the version in package.json and add an entry to the changelog.md

Done — version bumped to 9.6.2 in package.json and a changelog entry added for PR #1701.

@johnnyreilly johnnyreilly marked this pull request as ready for review June 19, 2026 17:29
@johnnyreilly

Copy link
Copy Markdown
Member

@copilot please give this PR a careful review - highlight any issues that might exist

Copilot AI commented Jun 19, 2026

Copy link
Copy Markdown
Contributor Author

@copilot please give this PR a careful review - highlight any issues that might exist

Review complete. One real bug was found and fixed; two other observations below.

Fixed — performance bug in formatErrors (src/utils.ts):
The reduce pattern-split and both picomatch() compilations were inside the .filter() callback, so patterns were recompiled on every diagnostic call. I hoisted them above the filter so they compile once per formatErrors invocation regardless of how many diagnostics are present. Committed in the latest push.

Observation — dead else branch in compilerSetup.ts:
After removing the semver.gte guard, the condition is now just if (compiler!.version !== undefined). Since TypeScript always exposes a version string, the else branch that logs "This version is incompatible" can never be reached. It's harmless dead code but worth noting — the branch could be removed entirely if desired.

Observation — correct behaviour confirmed:

  • All 80+ comparison tests pass, including reportFiles (both the standard and transpileOnly variants).
  • Picomatch 4.0.4 is CommonJS (type: cjs), so the import picomatch from 'picomatch' default-import compiles correctly under module: commonjs.
  • Windows path separators are handled correctly — picomatch 4 matches backslash paths against **/*.ts patterns without any extra normalisation needed.

@johnnyreilly

Copy link
Copy Markdown
Member

Please remove dead else branch @copilot

Copilot AI commented Jun 19, 2026

Copy link
Copy Markdown
Contributor Author

Please remove dead else branch @copilot

Done — the dead else branch (and its unreachable logError call) has been removed from getCompiler. compilerCompatible is now simply set to true directly when loaderOptions.compiler === 'typescript'.

Comment thread src/utils.ts Outdated
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

Successfully merging this pull request may close these issues.

2 participants