Skip to content

[WIP]feat(pack): library runtime code compitable with Array #2467

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
fireairforce wants to merge 1 commit into
base: next
Choose a base branch
from
Open

[WIP]feat(pack): library runtime code compitable with Array #2467

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 21 additions & 13 deletions crates/pack-core/js/src/umd/build-base.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,28 +47,31 @@ function instantiateModule(
sourceType: SourceType,
sourceData: SourceData,
): Module {
const moduleFactory = moduleFactories[id];
const moduleFactory = moduleFactories.get(id);
if (typeof moduleFactory !== "function") {
// This can happen if modules incorrectly handle HMR disposes/updates,
// e.g. when they keep a `setTimeout` around which still executes old code
// and contains e.g. a `require("something")` call.
factoryNotAvailable(id, sourceType, sourceData);
throw new Error(factoryNotAvailableMessage(id, sourceType, sourceData));
}

const module: Module = createModuleObject(id);
const exports = module.exports;

moduleCache[id] = module;

// NOTE(alexkirsz) This can fail when the module encounters a runtime error.
const context = new (Context as any as ContextConstructor<Module>)(
module,
exports,
);
try {
const context = new (Context as any as ContextConstructor<Module>)(module);
moduleFactory(context);
moduleFactory(context, module, exports);
} catch (error) {
module.error = error as any;
throw error;
}

module.loaded = true;
if (module.namespaceObject && module.exports !== module.namespaceObject) {
// in case of a circular dependency: cjs1 -> esm2 -> cjs1
interopEsm(module.exports, module.namespaceObject);
Expand All @@ -78,14 +81,19 @@ function instantiateModule(
}

// eslint-disable-next-line @typescript-eslint/no-unused-vars
function registerChunk([
chunkScript,
chunkModules,
runtimeParams,
]: ChunkRegistration) {
const chunkPath = getPathFromScript(chunkScript);
for (const [moduleId, moduleFactory] of Object.entries(chunkModules)) {
registerCompressedModuleFactory(moduleId, moduleFactory);
function registerChunk(registration: ChunkRegistration) {
const chunkPath = getPathFromScript(registration[0]);
let runtimeParams: RuntimeParams | undefined;
// When bootstrapping we are passed a single runtimeParams object so we can distinguish purely based on length
if (registration.length === 3) {
runtimeParams = registration[2] as RuntimeParams;
} else {
runtimeParams = undefined;
installCompressedModuleFactories(
registration as CompressedModuleFactories,
/* offset= */ 1,
moduleFactories,
);
}
Comment on lines +88 to 97
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Dec 29, 2025

Choose a reason for hiding this comment

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

critical

The logic in registerChunk is flawed. When registration.length === 3, it correctly extracts runtimeParams but fails to process registration[1], which contains the module factories. This will prevent any modules from being registered for chunks that have runtime parameters.

  if (registration.length === 3) {
    runtimeParams = registration[2] as RuntimeParams;
    installCompressedModuleFactories(
      registration[1] as CompressedModuleFactories,
      /* offset= */ 0,
      moduleFactories,
    );
  } else {
    runtimeParams = undefined;
    installCompressedModuleFactories(
      registration as CompressedModuleFactories,
      /* offset= */ 1,
      moduleFactories,
    );
  }


return BACKEND.registerChunk(chunkPath, runtimeParams);
Expand Down
30 changes: 5 additions & 25 deletions crates/pack-core/js/src/umd/runtime-base.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,7 @@ type RuntimeParams = {

type ChunkRegistration = [
chunkPath: ChunkScript,
chunkModules: CompressedModuleFactories,
params: RuntimeParams | undefined,
...(CompressedModuleFactories | [RuntimeParams]),
];
Comment on lines 43 to 46
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Dec 29, 2025

Choose a reason for hiding this comment

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

high

The type definition for ChunkRegistration appears to be invalid TypeScript. The spread operator ... cannot be applied to a union of an array type and a tuple type like CompressedModuleFactories | [RuntimeParams]. A simpler tuple type would be more appropriate and correct, reflecting that params is optional.

Suggested change
type ChunkRegistration = [
chunkPath: ChunkScript,
chunkModules: CompressedModuleFactories,
params: RuntimeParams | undefined,
...(CompressedModuleFactories | [RuntimeParams]),
];
type ChunkRegistration = [
chunkPath: ChunkScript,
chunkModules: CompressedModuleFactories,
params?: RuntimeParams,
];


type ChunkList = {
Expand Down Expand Up @@ -85,18 +84,18 @@ interface RuntimeBackend {
) => Promise<void>;
}

const moduleFactories: ModuleFactories = Object.create(null);
const moduleFactories: ModuleFactories = new Map();
contextPrototype.M = moduleFactories;

const availableModules: Map<ModuleId, Promise<any> | true> = new Map();

const availableModuleChunks: Map<ChunkPath, Promise<any> | true> = new Map();

function factoryNotAvailable(
function factoryNotAvailableMessage(
moduleId: ModuleId,
sourceType: SourceType,
sourceData: SourceData,
) {
): string {
let instantiationReason;
switch (sourceType) {
case SourceType.Runtime:
Expand All @@ -114,9 +113,7 @@ function factoryNotAvailable(
(sourceType) => `Unknown source type: ${sourceType}`,
);
}
throw new Error(
`Module ${moduleId} was instantiated ${instantiationReason}, but the module factory is not available. It might have been deleted in an HMR update.`,
);
return `Module ${moduleId} was instantiated ${instantiationReason}, but the module factory is not available.`;
}

const loadedChunk = Promise.resolve(undefined);
Expand Down Expand Up @@ -259,23 +256,6 @@ function getPathFromScript(
return path as ChunkPath | ChunkListPath;
}

function registerCompressedModuleFactory(
moduleId: ModuleId,
moduleFactory: Function | [Function, ModuleId[]],
) {
if (!moduleFactories[moduleId]) {
if (Array.isArray(moduleFactory)) {
let [moduleFactoryFn, otherIds] = moduleFactory;
moduleFactories[moduleId] = moduleFactoryFn;
for (const otherModuleId of otherIds) {
moduleFactories[otherModuleId] = moduleFactoryFn;
}
} else {
moduleFactories[moduleId] = moduleFactory;
}
}
}

const regexJsUrl = /\.js(?:\?[^#]*)?(?:#.*)?$/;
/**
* Checks if a given path/URL ends with .js, optionally followed by ?query or #fragment.
Expand Down
17 changes: 8 additions & 9 deletions crates/pack-core/js/src/umd/runtime-types.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,9 @@ type EsmImport = (
) => EsmNamespaceObject | Promise<EsmNamespaceObject>;
type InvokeAsyncLoader = (moduleId: ModuleId) => Promise<Exports>;
type EsmExport = (
exportGetters: Record<string, () => any>,
bindings: Array<
string | BindingTag | (() => unknown) | ((v: unknown) => void) | unknown
>,
id: ModuleId | undefined,
) => void;
type ExportValue = (value: any, id: ModuleId | undefined) => void;
Expand All @@ -56,12 +58,11 @@ type LoadChunkByUrl = (chunkUrl: ChunkUrl) => Promise<any> | undefined;

type ModuleCache<M> = Record<ModuleId, M>;
// TODO properly type values here
type ModuleFactories = Record<ModuleId, Function>;
// The value is an array with scope hoisting
type CompressedModuleFactories = Record<
ModuleId,
Function | [Function, ModuleId[]]
>;
type ModuleFactories = Map<ModuleId, Function>;
// This is an alternating, non-empty module factory functions and module ids
// [id1, id2..., factory1, id3, factory2, id4, id5, factory3]
// There are multiple ids to support scope hoisting modules
type CompressedModuleFactories = Array<ModuleId | Function>;

type RelativeURL = (inputUrl: string) => void;
type ResolvePathFromModule = (moduleId: string) => string;
Expand Down Expand Up @@ -91,13 +92,11 @@ type ExternalImport = (
interface Module {
exports: Function | Exports | Promise<Exports> | AsyncModulePromise;
error: Error | undefined;
loaded: boolean;
id: ModuleId;
namespaceObject?:
| EsmNamespaceObject
| Promise<EsmNamespaceObject>
| AsyncModulePromise<EsmNamespaceObject>;
[REEXPORTED_OBJECTS]?: any[];
}

interface ModuleWithDirection extends Module {
Expand Down
Loading
Loading