Skip to content

feat: accept a precompiled check in the worker's getOptions - #261

Open
alexander-akait wants to merge 2 commits into
mainfrom
feat/getoptions-precompiled-check
Open

alexander-akait wants to merge 2 commits into
mainfrom
feat/getoptions-precompiled-check

Conversation

@alexander-akait

@alexander-akait alexander-akait commented Oct 18, 2026 •

Copy link
Copy Markdown
Member

Summary

webpack/webpack#22745 adds an optional second argument to this.getOptions: this.getOptions(schema, check). When the precompiled check passes, webpack skips the schema, so schema-utils and ajv aren't loaded. Loaders are starting to pass it (webpack/coffee-loader#240, webpack/postcss-loader#746).

The worker reimplements getOptions, so a loader running under thread-loader would still compile its schema in every worker. The worker's getOptions now takes the same second argument:

  • A passing check returns the options without running the schema.
  • A failing check, or no check, validates as before, with the same error messages.

Tests

test/options-check.test.js runs a loader through a real worker:

  • When the check passes, options that the schema would reject come back without an error. This test fails without the change in src/worker.js.
  • When the check fails, the full validation error is reported.

npm run lint and the full suite (17 tests, 6 snapshots) pass locally.


Generated by Claude Code

Summary by CodeRabbit

  • New Features
    • Loader option handling now supports an optional check that can approve parsed options without schema validation. If the check is missing or does not approve the options, the existing schema validation still applies.

webpack's getOptions(schema, check) skips the schema when the check
passes (webpack/webpack#22745). The worker reimplements getOptions, so
a loader running under thread-loader kept compiling its schema. It now
takes the same second argument; a failing check still validates fully.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QYPkbvwyRWt2SHgXDoYRdo
@coderabbitai

coderabbitai Bot commented Oct 18, 2026

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 8b509bc4-9812-4e04-8796-756bfa351e1b
📥 Commits

Reviewing files that changed from the base of the PR and between 95079c4 and d0f9eae.

📒 Files selected for processing: 4
  • src/worker.js
  • test/options-check.test.js
  • test/options-check/file.js
  • test/options-check/options-check-loader.js

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

Walkthrough

The loader-context getOptions method now accepts an optional check callback. When a schema is present, it skips schema validation only if check is a function and returns a truthy value for the options. Otherwise, it runs schema validation. New webpack tests exercise trusted options and an invalid numeric name through a warmed worker.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to d0f9e

No actionable issue is identified with the optional check or its validation fallback; the change is mergeable after normal checks.

Pre-merge checks: Passed 5
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: adding support for a precompiled check in the worker's getOptions method.
Docstring Coverage Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 4…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 4 files.

✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Oct 18, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 14.75%. Comparing base (95079c4) to head (d537f4e).

Files with missing lines Patch % Lines
src/worker.js 0.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #261   +/-   ##
=======================================
  Coverage   14.75%   14.75%           
=======================================
  Files           9        9           
  Lines         732      732           
  Branches      166      166           
=======================================
  Hits          108      108           
  Misses        543      543           
  Partials       81       81           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

fast-uri <= 3.1.7 has high severity advisories (GHSA-v2hh-gcrm-f6hx and
others), which fail `npm audit --omit=dev` in the lint job. 3.1.8 is
within ajv's ^3.0.1 range, so only the lockfile changes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QYPkbvwyRWt2SHgXDoYRdo

Copy link
Copy Markdown
Member Author

codecov/patch reports 0% because src/worker.js runs in a separate worker process, and Jest only collects coverage in its own process. test/options-check.test.js exercises both new branches through a real worker (the first test fails without this change). Project coverage is unchanged at 14.75%.

The lint job's npm audit --omit=dev failure came from fast-uri 3.0.1 and fails on main too. #262 bumps it to 3.1.8, and the same commit is included here.


Generated by Claude Code

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.

1 participant