Skip to content

Experiment with adding webpack 4 support back#1697

Open
Copilot wants to merge 46 commits into
mainfrom
copilot/support-webpack-4-and-5
Open

Experiment with adding webpack 4 support back#1697
Copilot wants to merge 46 commits into
mainfrom
copilot/support-webpack-4-and-5

Conversation

Copy link
Copy Markdown

Copilot AI commented May 21, 2026

  • Review new @copilot comments and classify required actions
  • Inspect latest CI workflow runs and failing comparison jobs
  • Identify the first-breaking change and apply minimal fix/revert
  • Run targeted comparison tests and lint/build validation
  • Reply on PR comment with findings and commit hash

Agent-Logs-Url: https://github.com/TypeStrong/ts-loader/sessions/74b74f2d-76ad-4022-9635-77d0c5acce76

Co-authored-by: johnnyreilly <1010525+johnnyreilly@users.noreply.github.com>
Agent-Logs-Url: https://github.com/TypeStrong/ts-loader/sessions/e322b7ed-e010-4d60-9a2d-3672c9d55324

Co-authored-by: johnnyreilly <1010525+johnnyreilly@users.noreply.github.com>
@johnnyreilly
Copy link
Copy Markdown
Member

@copilot please consistently use instance.isWebpack5 to decide strategy

@johnnyreilly johnnyreilly changed the title Align webpack support metadata with current CI coverage Experiment with adding webpack 4 support back May 21, 2026
Copy link
Copy Markdown
Author

Copilot AI commented May 21, 2026

@copilot please consistently use instance.isWebpack5 to decide strategy

Done in 5d7ab2f. I updated the remaining strategy checks to consistently branch on instance.isWebpack5 (including cached/reused instance paths and build-dependency registration) rather than using direct feature detection at those decision points.

Comment thread src/utils.ts Outdated
} from './interfaces';
import { getInputFileNameFromOutput } from './instances';

export const isWebpack5 = !!(webpack as any).sources;
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.

Is this necessary @copilot?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Good catch — no, it wasn’t necessary there. I removed isWebpack5 from utils.ts and localized detection to instance setup in instances.ts in 78fa91f, so strategy still flows through instance.isWebpack5 without the extra utils export.

Copilot AI requested a review from johnnyreilly May 21, 2026 19:51
Comment thread src/after-compile.ts Outdated
(error: any) => error.loaderSource !== tsLoaderSource(loaderOptions)
)
.forEach(error => module.addError(error));
const webpackModule = module as any;
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 I don't think the webpack5 code needs to use webpackModule it can use module

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Updated in 7b3cd3e — webpack 5 path now uses module directly (getWarnings/getErrors/clearWarningsAndErrors) and keeps webpackModule only for the webpack 4 branch.

Copy link
Copy Markdown
Author

Copilot AI commented May 21, 2026

@copilot please consistently use instance.isWebpack5 to decide strategy

Handled in 5d7ab2f and retained in follow-ups: strategy decisions branch on instance.isWebpack5 consistently.

Copilot AI requested a review from johnnyreilly May 21, 2026 20:12
Copilot AI requested a review from johnnyreilly May 23, 2026 16:38
@johnnyreilly
Copy link
Copy Markdown
Member

@copilot please update CHANGELOG.md with this PR. This is a new feature so use that to determine version number

Copilot AI and others added 2 commits May 23, 2026 17:42
Copy link
Copy Markdown
Author

Copilot AI commented May 23, 2026

@copilot please update CHANGELOG.md with this PR. This is a new feature so use that to determine version number

Updated in c8be7fc. I added a new 9.6.0 entry to CHANGELOG.md for this feature PR (#1697) to reflect a minor-version feature release.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • accounts.google.com
    • Triggering command: /proc/self/exe /proc/self/exe --type=utility --utility-sub-type=network.mojom.NetworkService --lang=en-US --service-sandbox-type=none --no-sandbox --disable-dev-shm-usage --use-angle=swiftshader-webgl --crashpad-handler-pid=6179 --enable-crash-reporter=, --noerrdialogs --user-data-dir=/tmp/karma-5922743 --change-stack-guard-on-fork=enable --shared-files=v8_context_snapshot_data:100 --field-trial-handle=3,i,5957626381936697387,15937501044013309353,262144 --disable-features=PaintHolding --variations-seed-version --pseudonymization-salt-handle=7,i,2626899877038691307,4781234957290655775,4 --trace-process-track-uuid=3190708989122997041 (dns block)
    • Triggering command: /opt/google/chrome/chrome /usr/bin/google-chrome --user-data-dir=/tmp/karma-5922743 --enable-automation --no-default-browser-check --no-first-run --disable-default-apps --disable-popup-blocking --disable-translate --disable-REDACTED-timer-throttling --disable-renderer-REDACTEDing --disable-device-discovery-notifications --no-sandbox http://localhost:9876/?id=5922743 --headless --disable-gpu --disable-dev-shm-usage --remote-debugging-port=9222 (dns block)
    • Triggering command: /proc/self/exe /proc/self/exe --type=utility --utility-sub-type=network.mojom.NetworkService --lang=en-US --service-sandbox-type=none --no-sandbox --disable-dev-shm-usage --use-angle=swiftshader-webgl --crashpad-handler-pid=6329 --enable-crash-reporter=, --noerrdialogs --user-data-dir=/tmp/karma-55759591 --change-stack-guard-on-fork=enable --shared-files=v8_context_snapshot_data:100 --field-trial-handle=3,i,4824958347681221317,12510339631776023203,262144 --disable-features=PaintHolding --variations-seed-version --pseudonymization-salt-handle=7,i,8088342614959474917,13539472586258360379,4 --trace-process-track-uuid=3190708989122997041 (dns block)
  • clients2.google.com
    • Triggering command: /proc/self/exe /proc/self/exe --type=utility --utility-sub-type=network.mojom.NetworkService --lang=en-US --service-sandbox-type=none --no-sandbox --disable-dev-shm-usage --use-angle=swiftshader-webgl --crashpad-handler-pid=6179 --enable-crash-reporter=, --noerrdialogs --user-data-dir=/tmp/karma-5922743 --change-stack-guard-on-fork=enable --shared-files=v8_context_snapshot_data:100 --field-trial-handle=3,i,5957626381936697387,15937501044013309353,262144 --disable-features=PaintHolding --variations-seed-version --pseudonymization-salt-handle=7,i,2626899877038691307,4781234957290655775,4 --trace-process-track-uuid=3190708989122997041 (dns block)
    • Triggering command: /opt/google/chrome/chrome /usr/bin/google-chrome --user-data-dir=/tmp/karma-5922743 --enable-automation --no-default-browser-check --no-first-run --disable-default-apps --disable-popup-blocking --disable-translate --disable-REDACTED-timer-throttling --disable-renderer-REDACTEDing --disable-device-discovery-notifications --no-sandbox http://localhost:9876/?id=5922743 --headless --disable-gpu --disable-dev-shm-usage --remote-debugging-port=9222 (dns block)
    • Triggering command: /proc/self/exe /proc/self/exe --type=utility --utility-sub-type=network.mojom.NetworkService --lang=en-US --service-sandbox-type=none --no-sandbox --disable-dev-shm-usage --use-angle=swiftshader-webgl --crashpad-handler-pid=6329 --enable-crash-reporter=, --noerrdialogs --user-data-dir=/tmp/karma-55759591 --change-stack-guard-on-fork=enable --shared-files=v8_context_snapshot_data:100 --field-trial-handle=3,i,4824958347681221317,12510339631776023203,262144 --disable-features=PaintHolding --variations-seed-version --pseudonymization-salt-handle=7,i,8088342614959474917,13539472586258360379,4 --trace-process-track-uuid=3190708989122997041 (dns block)
  • clientservices.googleapis.com
    • Triggering command: /proc/self/exe /proc/self/exe --type=utility --utility-sub-type=network.mojom.NetworkService --lang=en-US --service-sandbox-type=none --no-sandbox --disable-dev-shm-usage --use-angle=swiftshader-webgl --crashpad-handler-pid=6179 --enable-crash-reporter=, --noerrdialogs --user-data-dir=/tmp/karma-5922743 --change-stack-guard-on-fork=enable --shared-files=v8_context_snapshot_data:100 --field-trial-handle=3,i,5957626381936697387,15937501044013309353,262144 --disable-features=PaintHolding --variations-seed-version --pseudonymization-salt-handle=7,i,2626899877038691307,4781234957290655775,4 --trace-process-track-uuid=3190708989122997041 (dns block)
    • Triggering command: /opt/google/chrome/chrome /usr/bin/google-chrome --user-data-dir=/tmp/karma-5922743 --enable-automation --no-default-browser-check --no-first-run --disable-default-apps --disable-popup-blocking --disable-translate --disable-REDACTED-timer-throttling --disable-renderer-REDACTEDing --disable-device-discovery-notifications --no-sandbox http://localhost:9876/?id=5922743 --headless --disable-gpu --disable-dev-shm-usage --remote-debugging-port=9222 (dns block)
    • Triggering command: /proc/self/exe /proc/self/exe --type=utility --utility-sub-type=network.mojom.NetworkService --lang=en-US --service-sandbox-type=none --no-sandbox --disable-dev-shm-usage --use-angle=swiftshader-webgl --crashpad-handler-pid=6329 --enable-crash-reporter=, --noerrdialogs --user-data-dir=/tmp/karma-55759591 --change-stack-guard-on-fork=enable --shared-files=v8_context_snapshot_data:100 --field-trial-handle=3,i,4824958347681221317,12510339631776023203,262144 --disable-features=PaintHolding --variations-seed-version --pseudonymization-salt-handle=7,i,8088342614959474917,13539472586258360379,4 --trace-process-track-uuid=3190708989122997041 (dns block)
  • redirector.gvt1.com
    • Triggering command: /proc/self/exe /proc/self/exe --type=utility --utility-sub-type=network.mojom.NetworkService --lang=en-US --service-sandbox-type=none --no-sandbox --disable-dev-shm-usage --use-angle=swiftshader-webgl --crashpad-handler-pid=6179 --enable-crash-reporter=, --noerrdialogs --user-data-dir=/tmp/karma-5922743 --change-stack-guard-on-fork=enable --shared-files=v8_context_snapshot_data:100 --field-trial-handle=3,i,5957626381936697387,15937501044013309353,262144 --disable-features=PaintHolding --variations-seed-version --pseudonymization-salt-handle=7,i,2626899877038691307,4781234957290655775,4 --trace-process-track-uuid=3190708989122997041 (dns block)
    • Triggering command: /opt/google/chrome/chrome /usr/bin/google-chrome --user-data-dir=/tmp/karma-5922743 --enable-automation --no-default-browser-check --no-first-run --disable-default-apps --disable-popup-blocking --disable-translate --disable-REDACTED-timer-throttling --disable-renderer-REDACTEDing --disable-device-discovery-notifications --no-sandbox http://localhost:9876/?id=5922743 --headless --disable-gpu --disable-dev-shm-usage --remote-debugging-port=9222 (dns block)
    • Triggering command: /proc/self/exe /proc/self/exe --type=utility --utility-sub-type=network.mojom.NetworkService --lang=en-US --service-sandbox-type=none --no-sandbox --disable-dev-shm-usage --use-angle=swiftshader-webgl --crashpad-handler-pid=6329 --enable-crash-reporter=, --noerrdialogs --user-data-dir=/tmp/karma-55759591 --change-stack-guard-on-fork=enable --shared-files=v8_context_snapshot_data:100 --field-trial-handle=3,i,4824958347681221317,12510339631776023203,262144 --disable-features=PaintHolding --variations-seed-version --pseudonymization-salt-handle=7,i,8088342614959474917,13539472586258360379,4 --trace-process-track-uuid=3190708989122997041 (dns block)
  • safebrowsingohttpgateway.googleapis.com
    • Triggering command: /proc/self/exe /proc/self/exe --type=utility --utility-sub-type=network.mojom.NetworkService --lang=en-US --service-sandbox-type=none --no-sandbox --disable-dev-shm-usage --use-angle=swiftshader-webgl --crashpad-handler-pid=6179 --enable-crash-reporter=, --noerrdialogs --user-data-dir=/tmp/karma-5922743 --change-stack-guard-on-fork=enable --shared-files=v8_context_snapshot_data:100 --field-trial-handle=3,i,5957626381936697387,15937501044013309353,262144 --disable-features=PaintHolding --variations-seed-version --pseudonymization-salt-handle=7,i,2626899877038691307,4781234957290655775,4 --trace-process-track-uuid=3190708989122997041 (dns block)
    • Triggering command: /opt/google/chrome/chrome /usr/bin/google-chrome --user-data-dir=/tmp/karma-5922743 --enable-automation --no-default-browser-check --no-first-run --disable-default-apps --disable-popup-blocking --disable-translate --disable-REDACTED-timer-throttling --disable-renderer-REDACTEDing --disable-device-discovery-notifications --no-sandbox http://localhost:9876/?id=5922743 --headless --disable-gpu --disable-dev-shm-usage --remote-debugging-port=9222 (dns block)
    • Triggering command: /proc/self/exe /proc/self/exe --type=utility --utility-sub-type=network.mojom.NetworkService --lang=en-US --service-sandbox-type=none --no-sandbox --disable-dev-shm-usage --use-angle=swiftshader-webgl --crashpad-handler-pid=6329 --enable-crash-reporter=, --noerrdialogs --user-data-dir=/tmp/karma-55759591 --change-stack-guard-on-fork=enable --shared-files=v8_context_snapshot_data:100 --field-trial-handle=3,i,4824958347681221317,12510339631776023203,262144 --disable-features=PaintHolding --variations-seed-version --pseudonymization-salt-handle=7,i,8088342614959474917,13539472586258360379,4 --trace-process-track-uuid=3190708989122997041 (dns block)
  • www.google.com
    • Triggering command: /proc/self/exe /proc/self/exe --type=utility --utility-sub-type=network.mojom.NetworkService --lang=en-US --service-sandbox-type=none --no-sandbox --disable-dev-shm-usage --use-angle=swiftshader-webgl --crashpad-handler-pid=6179 --enable-crash-reporter=, --noerrdialogs --user-data-dir=/tmp/karma-5922743 --change-stack-guard-on-fork=enable --shared-files=v8_context_snapshot_data:100 --field-trial-handle=3,i,5957626381936697387,15937501044013309353,262144 --disable-features=PaintHolding --variations-seed-version --pseudonymization-salt-handle=7,i,2626899877038691307,4781234957290655775,4 --trace-process-track-uuid=3190708989122997041 (dns block)
    • Triggering command: /opt/google/chrome/chrome /usr/bin/google-chrome --user-data-dir=/tmp/karma-5922743 --enable-automation --no-default-browser-check --no-first-run --disable-default-apps --disable-popup-blocking --disable-translate --disable-REDACTED-timer-throttling --disable-renderer-REDACTEDing --disable-device-discovery-notifications --no-sandbox http://localhost:9876/?id=5922743 --headless --disable-gpu --disable-dev-shm-usage --remote-debugging-port=9222 (dns block)
    • Triggering command: /proc/self/exe /proc/self/exe --type=utility --utility-sub-type=network.mojom.NetworkService --lang=en-US --service-sandbox-type=none --no-sandbox --disable-dev-shm-usage --use-angle=swiftshader-webgl --crashpad-handler-pid=6329 --enable-crash-reporter=, --noerrdialogs --user-data-dir=/tmp/karma-55759591 --change-stack-guard-on-fork=enable --shared-files=v8_context_snapshot_data:100 --field-trial-handle=3,i,4824958347681221317,12510339631776023203,262144 --disable-features=PaintHolding --variations-seed-version --pseudonymization-salt-handle=7,i,8088342614959474917,13539472586258360379,4 --trace-process-track-uuid=3190708989122997041 (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Comment thread src/after-compile.ts Outdated
);
}

function moduleHasErrors(module: webpack.Module, isWebpack5: boolean) {
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 the main branch doesn't have moduleHasErrors - why do we need it now?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

main is webpack-5-only, so it doesn’t need this guard. In this PR we reintroduced webpack 4 handling, where module errors are managed via mutable module.errors / module.warnings arrays and can already contain non-ts-loader errors. Without moduleHasErrors, ts-loader diagnostics get re-added at module level on top of existing errors, which changes/duplicates comparison output. We still always add diagnostics to compilation.errors; this guard only prevents module-level double-reporting in the webpack 4 path.

@johnnyreilly johnnyreilly force-pushed the copilot/support-webpack-4-and-5 branch from b5a3095 to 51e9fa2 Compare May 24, 2026 07:15
@johnnyreilly
Copy link
Copy Markdown
Member

@copilot yarn comparison-tests --single-test nodeModulesMeaningfulErrorWhenImportingTs succeeds on my machine but fails in CI - please investigate

Copy link
Copy Markdown
Author

Copilot AI commented May 24, 2026

@copilot yarn comparison-tests --single-test nodeModulesMeaningfulErrorWhenImportingTs succeeds on my machine but fails in CI - please investigate

Investigated and fixed in 7e6d119. CI logs showed nodeModulesMeaningfulErrorWhenImportingTs failing on output normalization around module error attribution. I updated after-compile to use runtime module capabilities when determining module resource/error paths (instead of assuming a single webpack API shape), and tagged the no-output loader error in index.ts so ts-loader-owned module errors are consistently filtered/re-added. I re-ran yarn build, yarn lint, and yarn comparison-tests -- --single-test nodeModulesMeaningfulErrorWhenImportingTs (plus errors) successfully.

@johnnyreilly
Copy link
Copy Markdown
Member

@copilot after the last commit all yarn comparison-tests are failing

Copy link
Copy Markdown
Author

Copilot AI commented May 24, 2026

@copilot after the last commit all yarn comparison-tests are failing

Fixed in 90b2860. I checked the failing CI jobs and they were failing before test execution with TS2416 type errors from @types/eslint-scope after the last lockfile change. I restored the prior lockfile resolution (the last commit had collapsed @types/eslint@*), which unblocked comparison test compilation. I then re-ran yarn build, yarn lint, yarn comparison-tests -- --single-test errors, and yarn comparison-tests -- --single-test nodeModulesMeaningfulErrorWhenImportingTs successfully.

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