Skip to content

Commit

Permalink
refactor(types): remove @ts-expect-error in loaderRunner and StatsFac…
Browse files Browse the repository at this point in the history
…tory (#7864)

* chore: ts

* chore: remove more ts-expect-error

* chore: adopt suggestions

* chore: refactor getResolve type
  • Loading branch information
SoonIter authored Sep 13, 2024
1 parent 23aa157 commit f3eb7d5
Show file tree
Hide file tree
Showing 11 changed files with 169 additions and 131 deletions.
24 changes: 15 additions & 9 deletions packages/rspack/etc/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -3295,7 +3295,7 @@ export class EnvironmentPlugin {
}

// @public (undocumented)
type ErrorWithDetail = Error & {
type ErrorWithDetails = Error & {
details?: string;
};

Expand Down Expand Up @@ -4511,10 +4511,10 @@ interface GlobalPassOption {
type GotHandler<T = any> = (result: any | null, callback: (error: Error | null) => void) => void;

// @public (undocumented)
type GroupConfig = {
type GroupConfig<T, R = T> = {
getKeys: (arg0: any) => string[] | undefined;
createGroup: <T, R>(arg0: string, arg1: (T | R)[], arg2: T[]) => R;
getOptions?: (<T>(arg0: string, arg1: T[]) => GroupOptions) | undefined;
createGroup: (key: string, arg1: (T | R)[], arg2: T[]) => R;
getOptions?: ((key: string, arg1: T[]) => GroupOptions) | undefined;
};

// @public
Expand Down Expand Up @@ -4577,7 +4577,7 @@ type Hooks = Readonly<{
extract: HookMap<SyncBailHook<[Object, any, StatsFactoryContext], undefined>>;
filter: HookMap<SyncBailHook<[any, StatsFactoryContext, number, number], undefined>>;
filterSorted: HookMap<SyncBailHook<[any, StatsFactoryContext, number, number], undefined>>;
groupResults: HookMap<SyncBailHook<[GroupConfig[], StatsFactoryContext], undefined>>;
groupResults: HookMap<SyncBailHook<[GroupConfig<any>[], StatsFactoryContext], undefined>>;
filterResults: HookMap<SyncBailHook<[any, StatsFactoryContext, number, number], undefined>>;
sort: HookMap<SyncBailHook<[
((arg1: any, arg2: any) => number)[],
Expand Down Expand Up @@ -5892,11 +5892,11 @@ export interface LoaderContext<OptionsType = {}> {
// (undocumented)
addMissingDependency(missing: string): void;
// (undocumented)
async(): (err?: Error | null, content?: string | Buffer, sourceMap?: string | SourceMap, additionalData?: AdditionalData) => void;
async(): LoaderContextCallback;
// (undocumented)
cacheable(cacheable?: boolean): void;
// (undocumented)
callback(err?: Error | null, content?: string | Buffer, sourceMap?: string | SourceMap, additionalData?: AdditionalData): void;
callback: LoaderContextCallback;
// (undocumented)
clearDependencies(): void;
// (undocumented)
Expand Down Expand Up @@ -5930,7 +5930,7 @@ export interface LoaderContext<OptionsType = {}> {
// (undocumented)
getOptions(schema?: any): OptionsType;
// (undocumented)
getResolve(options: Resolve): (context: any, request: any, callback: any) => Promise<any>;
getResolve(options: Resolve): ((context: string, request: string, callback: ResolveCallback) => void) | ((context: string, request: string) => Promise<string | false | undefined>);
// (undocumented)
hot?: boolean;
// (undocumented)
Expand Down Expand Up @@ -5980,6 +5980,9 @@ export interface LoaderContext<OptionsType = {}> {
version: 2;
}

// @public (undocumented)
type LoaderContextCallback = (err?: Error | null, content?: string | Buffer, sourceMap?: string | SourceMap, additionalData?: AdditionalData) => void;

// @public (undocumented)
export type LoaderDefinition<OptionsType = {}, ContextAdditions = {}> = LoaderDefinitionFunction<OptionsType, ContextAdditions> & {
raw?: false;
Expand Down Expand Up @@ -9821,6 +9824,9 @@ export type ResolveAlias = z.infer<typeof resolveAlias>;
// @public (undocumented)
const resolveAlias: z.ZodRecord<z.ZodString, z.ZodUnion<[z.ZodUnion<[z.ZodLiteral<false>, z.ZodString]>, z.ZodArray<z.ZodUnion<[z.ZodString, z.ZodLiteral<false>]>, "many">]>>;

// @public (undocumented)
type ResolveCallback = (err: null | ErrorWithDetails, res?: string | false, req?: ResolveRequest) => void;

// @public (undocumented)
type ResolveContext = {};

Expand Down Expand Up @@ -9861,7 +9867,7 @@ class Resolver {
// (undocumented)
binding: binding.JsResolver;
// (undocumented)
resolve(context: object, path: string, request: string, resolveContext: ResolveContext, callback: (err: null | ErrorWithDetail, res?: string | false) => void): void;
resolve(context: object, path: string, request: string, resolveContext: ResolveContext, callback: ResolveCallback): void;
// (undocumented)
resolveSync(context: object, path: string, request: string): string | false;
// (undocumented)
Expand Down
14 changes: 6 additions & 8 deletions packages/rspack/src/Resolver.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
import type * as binding from "@rspack/binding";
import { type Resolve, getRawResolve } from "./config";
import type {
ErrorWithDetails,
ResolveCallback
} from "./config/adapterRuleUse";

type ResolveContext = {};

type ErrorWithDetail = Error & { details?: string };

type ResolveOptionsWithDependencyType = Resolve & {
dependencyCategory?: string;
resolveToContext?: boolean;
Expand All @@ -30,17 +32,13 @@ export class Resolver {
path: string,
request: string,
resolveContext: ResolveContext,
callback: (
err: null | ErrorWithDetail,
res?: string | false
// req?: ResolveRequest
) => void
callback: ResolveCallback
): void {
try {
const res = this.binding.resolveSync(path, request);
callback(null, res);
} catch (err) {
callback(err as ErrorWithDetail);
callback(err as ErrorWithDetails);
}
}

Expand Down
37 changes: 24 additions & 13 deletions packages/rspack/src/config/adapterRuleUse.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,24 +50,30 @@ export interface AdditionalData {
[index: string]: any;
}

export type LoaderContextCallback = (
err?: Error | null,
content?: string | Buffer,
sourceMap?: string | SourceMap,
additionalData?: AdditionalData
) => void;

export type ErrorWithDetails = Error & { details?: string };

// aligned with https://github.com/webpack/webpack/blob/64e8e33151c3fabd3f1917851193e458a526e803/declarations/LoaderContext.d.ts#L19
export type ResolveCallback = (
err: null | ErrorWithDetails,
res?: string | false,
req?: ResolveRequest
) => void;

export interface LoaderContext<OptionsType = {}> {
version: 2;
resource: string;
resourcePath: string;
resourceQuery: string;
resourceFragment: string;
async(): (
err?: Error | null,
content?: string | Buffer,
sourceMap?: string | SourceMap,
additionalData?: AdditionalData
) => void;
callback(
err?: Error | null,
content?: string | Buffer,
sourceMap?: string | SourceMap,
additionalData?: AdditionalData
): void;
async(): LoaderContextCallback;
callback: LoaderContextCallback;
cacheable(cacheable?: boolean): void;
sourceMap: boolean;
rootContext: string;
Expand Down Expand Up @@ -114,7 +120,12 @@ export interface LoaderContext<OptionsType = {}> {
): void;
getResolve(
options: Resolve
): (context: any, request: any, callback: any) => Promise<any>;
):
| ((context: string, request: string, callback: ResolveCallback) => void)
| ((
context: string,
request: string
) => Promise<string | false | undefined>);
getLogger(name: string): Logger;
emitError(error: Error): void;
emitWarning(warning: Error): void;
Expand Down
59 changes: 28 additions & 31 deletions packages/rspack/src/loader-runner/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import { NonErrorEmittedError, type RspackError } from "../RspackError";
import {
BUILTIN_LOADER_PREFIX,
type LoaderContext,
type LoaderContextCallback,
isUseSimpleSourceMap,
isUseSourceMap
} from "../config/adapterRuleUse";
Expand Down Expand Up @@ -236,45 +237,44 @@ const runSyncOrAsync = promisify(function runSyncOrAsync(
fn: Function,
context: LoaderContext,
args: any[],
callback: (err: Error | null, args: any[]) => void
callback: (err: Error | null | undefined, args: any[]) => void
) {
let isSync = true;
let isDone = false;
let isError = false; // internal error
let reportedError = false;
// @ts-expect-error loader-runner leverages `arguments` to achieve the same functionality.
context.async = function async() {
if (isDone) {
if (reportedError) return; // ignore
if (reportedError) return undefined as any; // ignore
throw new Error("async(): The callback was already called.");
}
isSync = false;
return innerCallback;
};
const innerCallback = (context.callback = (err, ...args) => {
const innerCallback: LoaderContextCallback = (err, ...args) => {
if (isDone) {
if (reportedError) return; // ignore
throw new Error("callback(): The callback was already called.");
}
isDone = true;
isSync = false;
try {
// @ts-expect-error
callback(err, args);
} catch (e) {
isError = true;
throw e;
}
});
};
context.callback = innerCallback;

try {
const result = (function LOADER_EXECUTION() {
return fn.apply(context, args);
})();
if (isSync) {
isDone = true;
if (result === undefined) {
// @ts-expect-error
callback();
callback(null, []);
return;
}
if (
Expand Down Expand Up @@ -306,8 +306,7 @@ const runSyncOrAsync = promisify(function runSyncOrAsync(
}
isDone = true;
reportedError = true;
// @ts-expect-error
callback(e);
callback(e as Error, []);
}
});

Expand Down Expand Up @@ -585,26 +584,27 @@ export async function runLoaders(
loaderContext.resolve = function resolve(context, request, callback) {
resolver.resolve({}, context, request, getResolveContext(), callback);
};
// @ts-expect-error TODO

loaderContext.getResolve = function getResolve(options) {
const child = options ? resolver.withOptions(options as any) : resolver;
return (context, request, callback) => {
if (callback) {
child.resolve({}, context, request, getResolveContext(), callback);
} else {
return new Promise((resolve, reject) => {
child.resolve(
{},
context,
request,
getResolveContext(),
(err, result) => {
if (err) reject(err);
else resolve(result);
}
);
});
return;
}
// TODO: (type) our native resolver return value is "string | false" but webpack type is "string"
return new Promise<string | false | undefined>((resolve, reject) => {
child.resolve(
{},
context,
request,
getResolveContext(),
(err, result) => {
if (err) reject(err);
else resolve(result);
}
);
});
};
};
loaderContext.getLogger = function getLogger(name) {
Expand Down Expand Up @@ -651,7 +651,7 @@ export async function runLoaders(
sourceMap?,
assetInfo?
) {
let source: Source;
let source: Source | undefined = undefined;
if (sourceMap) {
if (
typeof sourceMap === "string" &&
Expand Down Expand Up @@ -679,13 +679,10 @@ export async function runLoaders(
content
);
}
// @ts-expect-error
compiler._lastCompilation.__internal__emit_asset_from_loader(
compiler._lastCompilation!.__internal__emit_asset_from_loader(
name,
// @ts-expect-error
source,
// @ts-expect-error
assetInfo,
source!,
assetInfo!,
context._moduleIdentifier
);
};
Expand Down
4 changes: 2 additions & 2 deletions packages/rspack/src/node/NodeEnvironmentPlugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@
* Copyright (c) JS Foundation and other contributors
* https://github.com/webpack/webpack/blob/main/LICENSE
*/
// @ts-expect-error
import CachedInputFileSystem from "enhanced-resolve/lib/CachedInputFileSystem";
// @ts-expect-error we directly import from enhanced-resolve inner js file to improve performance
import CachedInputFileSystem from "enhanced-resolve/lib/CachedInputFileSystem.js";
import fs from "graceful-fs";

import type { Compiler } from "..";
Expand Down
Loading

2 comments on commit f3eb7d5

@rspack-bot
Copy link

Choose a reason for hiding this comment

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

📝 Benchmark detail: Open

Name Base (2024-09-13 56a30b4) Current Change
10000_development-mode + exec 2.24 s ± 49 ms 2.22 s ± 22 ms -0.83 %
10000_development-mode_hmr + exec 692 ms ± 15 ms 690 ms ± 11 ms -0.26 %
10000_production-mode + exec 2.84 s ± 28 ms 2.86 s ± 40 ms +0.50 %
arco-pro_development-mode + exec 1.85 s ± 66 ms 1.86 s ± 69 ms +0.16 %
arco-pro_development-mode_hmr + exec 435 ms ± 2 ms 435 ms ± 2.8 ms +0.08 %
arco-pro_production-mode + exec 3.23 s ± 46 ms 3.26 s ± 43 ms +1.01 %
arco-pro_production-mode_generate-package-json-webpack-plugin + exec 3.31 s ± 61 ms 3.33 s ± 96 ms +0.54 %
threejs_development-mode_10x + exec 1.7 s ± 12 ms 1.7 s ± 14 ms +0.14 %
threejs_development-mode_10x_hmr + exec 802 ms ± 11 ms 805 ms ± 4.2 ms +0.32 %
threejs_production-mode_10x + exec 5.18 s ± 26 ms 5.19 s ± 16 ms +0.20 %

@rspack-bot
Copy link

Choose a reason for hiding this comment

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

📝 Ran ecosystem CI: Open

suite result
modernjs undefined null
_selftest undefined null
nx ❌ failure
rspress ✅ success
rslib undefined null
rsbuild undefined null
examples undefined null

Please sign in to comment.