Skip to content

docs(NormalModule): enhance JSDocs and typings for loaderContext#20888

Merged
alexander-akait merged 3 commits into
webpack:mainfrom
moshams272:docs/enhance-jsdocs
May 22, 2026
Merged

docs(NormalModule): enhance JSDocs and typings for loaderContext#20888
alexander-akait merged 3 commits into
webpack:mainfrom
moshams272:docs/enhance-jsdocs

Conversation

@moshams272

@moshams272 moshams272 commented Apr 28, 2026

Copy link
Copy Markdown
Contributor

Summary

This PR enhances and standardizes the JSDoc typings for the loaderContext methods in lib/NormalModule.js.

JSDoc descriptions were moved to declarations/LoaderContext.d.ts, and lib/NormalModule.js now uses type references to avoid duplication and keep the runtime code clean.

Part Of Issue: webpack/webpack-doc-kit#88

What kind of change does this PR introduce?

docs

Did you add tests for your changes?

No. This PR only updates JSDoc comments and type annotations

Does this PR introduce a breaking change?

No.

If relevant, what needs to be documented once your changes are merged or what have you already documented?

Nothing.

Use of AI

Yes. I used it to help format the JSDoc comments to match Webpack's internal coding style. However, all types and logic were manually traced and verified against the actual Webpack source code before submission.

@changeset-bot

changeset-bot Bot commented Apr 28, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 6900037

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Comment thread lib/NormalModule.js Outdated
@linux-foundation-easycla

linux-foundation-easycla Bot commented Apr 29, 2026

Copy link
Copy Markdown

CLA Signed

The committers listed above are authorized under a signed CLA.

@moshams272 moshams272 force-pushed the docs/enhance-jsdocs branch 2 times, most recently from 231ca88 to 7d67d6a Compare April 29, 2026 13:24
Comment thread lib/NormalModule.js Outdated
/** @type {AnyLoaderContext} */
(loaderContext)
);
getOptions: /** @type {LoaderContext<EXPECTED_ANY>["getOptions"]} */ (

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.

Please move it to top of getOptions no using as

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.

I actually tried doing this before the final commit, but the TS compiler throws an error (Target signature provides too few arguments) because getOptions is overloaded in LoaderContext.d.ts.

If we don't want to use casting, we can use one of these two ideas:

  • Merge the overloads into a single signature with an optional parameter like: getOptions(schema?: Schema): OptionsType;.
  • Keep the overloads, but add a default parameter to the runtime method getOptions: (schema = undefined) => { ... }.

any other suggestion?

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.

it means we need to fix types in this case

@moshams272 moshams272 Apr 29, 2026

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.

Yeah, if we use the first option.

However, I think of others overload signature functions that has completely different parameters and how we can handle this.

That's why I asked for suggestions.

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.

We should keep overloads, because they help to make less bugs, so feel free to

Keep the overloads, but add a default parameter to the runtime method getOptions: (schema = undefined) => { ... }.

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.

Added an EXPECTED_ANY type for schema cause it threw an error when accessing schema.title later in the function.

/** @type {LoaderContext<EXPECTED_ANY>["getOptions"]} */
			getOptions: (/** @type {EXPECTED_ANY} */ schema = undefined) => {
				const loader = this.getCurrentLoader(
					/** @type {AnyLoaderContext} */
					(loaderContext)
				);

				let ...

@moshams272 moshams272 force-pushed the docs/enhance-jsdocs branch from 7d67d6a to e6f2c27 Compare April 29, 2026 19:43
@alexander-akait

Copy link
Copy Markdown
Member

@moshams272 sorry for delay, can you rebase, thanks

@moshams272 moshams272 force-pushed the docs/enhance-jsdocs branch from e6f2c27 to b262323 Compare May 21, 2026 14:28
@moshams272

Copy link
Copy Markdown
Contributor Author

@moshams272 sorry for delay, can you rebase, thanks

NP, Done!

@codecov

codecov Bot commented May 21, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.56%. Comparing base (916810e) to head (6900037).
⚠️ Report is 9 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #20888      +/-   ##
==========================================
- Coverage   91.58%   91.56%   -0.03%     
==========================================
  Files         573      573              
  Lines       59277    59491     +214     
  Branches    16012    16065      +53     
==========================================
+ Hits        54289    54471     +182     
- Misses       4988     5020      +32     
Flag Coverage Δ
integration 89.47% <100.00%> (-0.11%) ⬇️
test262 45.34% <100.00%> (+2.04%) ⬆️
unit 37.95% <100.00%> (+0.09%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 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.

@codspeed-hq

codspeed-hq Bot commented May 21, 2026

Copy link
Copy Markdown

Merging this PR will degrade performance by 22.53%

⚡ 4 improved benchmarks
❌ 2 regressed benchmarks
✅ 138 untouched benchmarks
⏩ 72 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Memory benchmark "wasm-modules-async", scenario '{"name":"mode-development-rebuild","mode":"development","watch":true}' 394.9 KB 1,544.2 KB -74.43%
Memory benchmark "cache-filesystem", scenario '{"name":"mode-development-rebuild","mode":"development","watch":true}' 1,025.2 KB 840.6 KB +21.97%
Memory benchmark "css-modules", scenario '{"name":"mode-development","mode":"development"}' 1,082.6 KB 534.1 KB ×2
Memory benchmark "many-chunks-esm", scenario '{"name":"mode-development-rebuild","mode":"development","watch":true}' 168.9 KB 922.8 KB -81.7%
Memory benchmark "asset-modules-resource", scenario '{"name":"mode-development-rebuild","mode":"development","watch":true}' 526.6 KB 345.2 KB +52.56%
Memory benchmark "wasm-modules-sync", scenario '{"name":"mode-production","mode":"production"}' 7.7 MB 6.3 MB +22.44%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing moshams272:docs/enhance-jsdocs (6900037) with main (7dd1824)

Open in CodSpeed

Footnotes

  1. 72 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@alexander-akait

Copy link
Copy Markdown
Member

Need fix lint

@moshams272 moshams272 force-pushed the docs/enhance-jsdocs branch from b262323 to e042e5a Compare May 22, 2026 08:15
@alexander-akait

Copy link
Copy Markdown
Member

Lint failed

@alexander-akait alexander-akait merged commit 7f891b0 into webpack:main May 22, 2026
54 of 56 checks passed
@github-actions

Copy link
Copy Markdown
Contributor

This PR is packaged and the instant preview is available (7f891b0).

Install it locally:

  • npm
npm i -D webpack@https://pkg.pr.new/webpack@7f891b0
  • yarn
yarn add -D webpack@https://pkg.pr.new/webpack@7f891b0
  • pnpm
pnpm add -D webpack@https://pkg.pr.new/webpack@7f891b0

@moshams272 moshams272 deleted the docs/enhance-jsdocs branch May 22, 2026 12:57
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