docs(types): add jsdocs types#229
Merged
Merged
Conversation
…peScript Annotate every file in lib/ with JSDoc types that mirror the public types in tapable.d.ts. Add a tsconfig.json and a `build:types` script so .d.ts files can be generated from the JSDoc sources into ./types (gitignored). The public tapable.d.ts is unchanged so consumer types are unaffected. Use the EXPECTED_ANY / EXPECTED_FUNCTION typedefs (mirroring the eslint-config-webpack convention) to satisfy strict JSDoc rules. Adding typescript as a direct devDependency activates the project's strict JSDoc rules on README and benchmark files; restore prior lint behavior for those paths via narrow rule overrides.
|
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #229 +/- ##
==========================================
- Coverage 99.35% 99.25% -0.11%
==========================================
Files 15 15
Lines 775 805 +30
Branches 128 140 +12
==========================================
+ Hits 770 799 +29
- Misses 5 6 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
- Pass generic args to CompileOptions in all hook subclasses (needs 2-3 type args after upstream made it generic). - Default args parameter to a properly typed cast (via unknown -> ArgumentNames) instead of bare [] in Hook constructor and every subclass factory function. - Cast subclass factory return values to the declared Hook<T, R, AdditionalOptions> return type (via unknown), since `new Hook(...)` produces the default-parameter variance that does not assign to the generic return. - Fix typo in Hook.CompileOptions where `args` was incorrectly typed as `ArgumentNames<AsArray<T>>[]` (an array of arg-arrays); it should be `ArgumentNames<AsArray<T>>` to match the constructor signature. - Cast _createCall results inside CALL_DELEGATE / CALL_ASYNC_DELEGATE / PROMISE_DELEGATE so the `EXPECTED_FUNCTION` return is assignable to the delegate fields. - Widen mergeOptions parameter to `string | (TapOptions & IfSet<...>)` so the wrapped withOptions can recurse through it. - Cast `taps[i]` to `FullTap & IfSet<AdditionalOptions>` when calling `interceptor.register` in Hook#intercept. - Add MultiHook tap/tapAsync/tapPromise @template T,R declarations so the JSDoc types resolve, and cast withOptions / tapPromise call sites where the underlying hook contract is wider than the local generics. - Initialize `fn` as `EXPECTED_FUNCTION | undefined` and cast on return in HookCodeFactory#create; type the `every` callback param explicitly; cast options.args when assigning to the local `_args: string[]`.
Lock down the optimization that lives in `Hook.js`: the
`Object.setPrototypeOf(Hook.prototype, null)` call and the trailing
`this.compile = this.compile` (and friends) self-assignments in the
constructor. Together they force `compile`/`tap`/`tapAsync`/`tapPromise`
to be own properties in a fixed insertion order so that subclass
overrides (`SyncHook`, `AsyncSeriesHook`, ...) overwrite existing slots
instead of triggering a hidden-class transition - keeping shared call
sites monomorphic.
The new tests assert:
- Hook.prototype has a null prototype.
- compile / tap / tapAsync / tapPromise and the call/_call,
callAsync/_callAsync, promise/_promise pairs are own properties.
- The base Hook layout is the documented 15-key sequence.
- Every subclass instance shares the same 16-key layout (base + the
trailing `constructor` slot the factories add) - across all 10
subclass families.
- Exercising a hook (intercept/tap/call) does not introduce new own
properties or change ordering.
- Async* hooks keep `call`/`_call` as own properties holding
`undefined` rather than deleting them, so the layout still matches.
A future "cleanup" that removes the self-assignments or rearranges the
constructor will now fail the suite instead of silently regressing
hot-path performance.
The base `content` placeholder shadowed the subclass overrides on the prototype chain - every `this.content(options)` lookup in `contentWithInterceptors` had to walk past `HookCodeFactory.prototype` before hitting the concrete `SyncHookCodeFactory`/`AsyncSeriesHookCodeFactory` implementation. On the hot compile path that's a real cost in webpack where `compile()` runs for every hook on every plugin registration. Remove the stub so `content` exists only on subclass prototypes - V8 sees a single, monomorphic shape for the lookup. Use the `@ts-expect-error abstract method ... for performance reasons` pattern already established on `Hook.js` (CALL_DELEGATE etc.) to keep `lint:types` green at the two call sites. The trade-off: instantiating `HookCodeFactory` directly and calling `content` now throws `TypeError: this.content is not a function` instead of the prior friendlier `Error: Abstract: should be overridden`. The class is internal and never exposed; this is acceptable.
Each subclass factory (SyncHook, AsyncParallelHook, ...) was returning the generic `Hook<T, R, AdditionalOptions>` type, so consumers calling `new SyncHook()` got `Hook<...>` from JSDoc-generated types instead of the specific `SyncHook<...>` declared in the public `tapable.d.ts`. Mirror the public class declarations: each subclass file gets a typedef that re-exports the corresponding class from `tapable.d.ts` (with a `Type` suffix to avoid colliding with the factory function name in scope), then wires it as the factory's `@returns` and inner cast target. This keeps the V8 hidden-class optimization intact (still factory functions, no real `class extends Hook`) while aligning the JSDoc types with what tapable.d.ts already promises to consumers.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add jsdocs types to generate types
What kind of change does this PR introduce?
refactor
Did you add tests for your changes?
Existing
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, claude