Skip to content

(BREAKING) Change Webpack config to output real ESM modules #1393

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
ibacher wants to merge 1 commit into
base: main
Choose a base branch
from
Open

(BREAKING) Change Webpack config to output real ESM modules #1393

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
55 changes: 32 additions & 23 deletions packages/framework/esm-dynamic-loading/src/dynamic-loading.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,32 +42,41 @@ export async function importDynamic<T = any>(
maxLoadingTime?: number;
},
): Promise<T> {
// default to 10 minutes
const maxLoadingTime = !options?.maxLoadingTime || options.maxLoadingTime <= 0 ? 600_000 : options.maxLoadingTime;

let timeout: ReturnType<typeof setTimeout> | undefined = undefined;
await Promise.race([
preloadImport(jsPackage, options?.importMap),
new Promise((_, reject) => {
timeout = setTimeout(() => {
reject(
new Error(`Could not resolve requested script, ${jsPackage}, within ${humanReadableMs(maxLoadingTime)}.`),
);
}, maxLoadingTime);
}),
]);

timeout && clearTimeout(timeout);

const jsPackageSlug = slugify(jsPackage);
const maybeContainer = await import(/* webpackIgnore: true */ jsPackage);
Copy link
Member Author

@ibacher ibacher Jun 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Once we've migrated the existing apps, we should be able to replace this whole dynamic loading stuff with something like:

const container = await import(/* webpackIgnore: true */ jsPackage);
if (!isFederatedModule(container)) {
  const error = `The package ${jsPackage} is not a federated module`;
  console.error(error);
  throw new Error(error);
}

container.init(__webpack_share_scopes__.default);

const factory = await container.get(share);
const module = factory();

if (!(typeof module === 'object') || module === null) {
  const error = `Container for ${jsPackage} did not return an ESM module as expected`;
  console.error(error);
  throw new Error(error);
}
return module as unknown as T;

And once we get to shared dependencies on the import map, this will all go away completely.

if (maybeContainer && isFederatedModule(maybeContainer)) {
return processContainer<T>(jsPackage, maybeContainer, share);
} else {
// default to 10 minutes
const maxLoadingTime = !options?.maxLoadingTime || options.maxLoadingTime <= 0 ? 600_000 : options.maxLoadingTime;

let timeout: ReturnType<typeof setTimeout> | undefined = undefined;
await Promise.race([
preloadImport(jsPackage, options?.importMap),
new Promise((_, reject) => {
timeout = setTimeout(() => {
reject(
new Error(`Could not resolve requested script, ${jsPackage}, within ${humanReadableMs(maxLoadingTime)}.`),
);
}, maxLoadingTime);
}),
]);

timeout && clearTimeout(timeout);

const jsPackageSlug = slugify(jsPackage);

const container = window[jsPackageSlug] as unknown;
if (!isFederatedModule(container)) {
const error = `The global variable ${jsPackageSlug} does not refer to a federated module`;
console.error(error);
throw new Error(error);
}

const container = window[jsPackageSlug] as unknown;
if (!isFederatedModule(container)) {
const error = `The global variable ${jsPackageSlug} does not refer to a federated module`;
console.error(error);
throw new Error(error);
return processContainer<T>(jsPackage, container, share);
}
}

async function processContainer<T>(jsPackage: string, container: FederatedModule, share: string) {
container.init(__webpack_share_scopes__.default);

const factory = await container.get(share);
Expand Down
16 changes: 8 additions & 8 deletions packages/framework/esm-framework/docs/API.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -571,7 +571,7 @@ A definition of an extension as extracted from an app's routes.json

#### Defined in

packages/framework/esm-globals/dist/types.d.ts:171
packages/framework/esm-globals/dist/types.d.ts:177

___

Expand Down Expand Up @@ -601,7 +601,7 @@ A definition of a modal as extracted from an app's routes.json

#### Defined in

packages/framework/esm-globals/dist/types.d.ts:234
packages/framework/esm-globals/dist/types.d.ts:240

___

Expand All @@ -611,7 +611,7 @@ ___

#### Defined in

packages/framework/esm-globals/dist/types.d.ts:392
packages/framework/esm-globals/dist/types.d.ts:398

___

Expand All @@ -624,7 +624,7 @@ Basically, this is the same as the app routes, with each routes definition keyed

#### Defined in

packages/framework/esm-globals/dist/types.d.ts:388
packages/framework/esm-globals/dist/types.d.ts:394

___

Expand All @@ -636,7 +636,7 @@ A definition of a page extracted from an app's routes.json

#### Defined in

packages/framework/esm-globals/dist/types.d.ts:113
packages/framework/esm-globals/dist/types.d.ts:119

___

Expand Down Expand Up @@ -690,7 +690,7 @@ ___

#### Defined in

packages/framework/esm-globals/dist/types.d.ts:73
packages/framework/esm-globals/dist/types.d.ts:79

___

Expand Down Expand Up @@ -810,7 +810,7 @@ A definition of a workspace as extracted from an app's routes.json

#### Defined in

packages/framework/esm-globals/dist/types.d.ts:264
packages/framework/esm-globals/dist/types.d.ts:270

___

Expand All @@ -820,7 +820,7 @@ ___

#### Defined in

packages/framework/esm-globals/dist/types.d.ts:260
packages/framework/esm-globals/dist/types.d.ts:266

___

Expand Down
6 changes: 3 additions & 3 deletions packages/framework/esm-framework/docs/interfaces/FeatureFlagDefinition.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ An explanation of what the flag does, which will be displayed in the Implementer

#### Defined in

packages/framework/esm-globals/dist/types.d.ts:353
packages/framework/esm-globals/dist/types.d.ts:359

___

Expand All @@ -34,7 +34,7 @@ A code-friendly name for the flag, which will be used to reference it in code

#### Defined in

packages/framework/esm-globals/dist/types.d.ts:349
packages/framework/esm-globals/dist/types.d.ts:355

___

Expand All @@ -46,4 +46,4 @@ A human-friendly name which will be displayed in the Implementer Tools

#### Defined in

packages/framework/esm-globals/dist/types.d.ts:351
packages/framework/esm-globals/dist/types.d.ts:357
2 changes: 1 addition & 1 deletion packages/framework/esm-framework/docs/interfaces/ImportMap.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,4 @@

#### Defined in

packages/framework/esm-globals/dist/types.d.ts:75
packages/framework/esm-globals/dist/types.d.ts:81
18 changes: 9 additions & 9 deletions packages/framework/esm-framework/docs/interfaces/OpenmrsAppRoutes.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ A list of backend modules necessary for this frontend module and the correspondi

#### Defined in

packages/framework/esm-globals/dist/types.d.ts:360
packages/framework/esm-globals/dist/types.d.ts:366

___

Expand All @@ -40,7 +40,7 @@ An array of all extensions supported by this frontend module. Extensions can be

#### Defined in

packages/framework/esm-globals/dist/types.d.ts:374
packages/framework/esm-globals/dist/types.d.ts:380

___

Expand All @@ -52,7 +52,7 @@ An array of all feature flags for any beta-stage features this module provides.

#### Defined in

packages/framework/esm-globals/dist/types.d.ts:376
packages/framework/esm-globals/dist/types.d.ts:382

___

Expand All @@ -64,7 +64,7 @@ An array of all modals supported by this frontend module. Modals can be launched

#### Defined in

packages/framework/esm-globals/dist/types.d.ts:378
packages/framework/esm-globals/dist/types.d.ts:384

___

Expand All @@ -82,7 +82,7 @@ The name of the backend dependency and either the required version or an object

#### Defined in

packages/framework/esm-globals/dist/types.d.ts:362
packages/framework/esm-globals/dist/types.d.ts:368

___

Expand All @@ -94,7 +94,7 @@ An array of all pages supported by this frontend module. Pages are automatically

#### Defined in

packages/framework/esm-globals/dist/types.d.ts:372
packages/framework/esm-globals/dist/types.d.ts:378

___

Expand All @@ -106,7 +106,7 @@ The version of this frontend module.

#### Defined in

packages/framework/esm-globals/dist/types.d.ts:358
packages/framework/esm-globals/dist/types.d.ts:364

___

Expand All @@ -118,7 +118,7 @@ An array of all workspace groups supported by this frontend module.

#### Defined in

packages/framework/esm-globals/dist/types.d.ts:382
packages/framework/esm-globals/dist/types.d.ts:388

___

Expand All @@ -130,4 +130,4 @@ An array of all workspaces supported by this frontend module. Workspaces can be

#### Defined in

packages/framework/esm-globals/dist/types.d.ts:380
packages/framework/esm-globals/dist/types.d.ts:386
2 changes: 1 addition & 1 deletion packages/framework/esm-framework/docs/interfaces/ResourceLoader.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,4 @@

#### Defined in

packages/framework/esm-globals/dist/types.d.ts:390
packages/framework/esm-globals/dist/types.d.ts:396
10 changes: 5 additions & 5 deletions packages/framework/esm-framework/docs/interfaces/SpaConfig.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ The base path or URL for the OpenMRS API / endpoints.

#### Defined in

packages/framework/esm-globals/dist/types.d.ts:84
packages/framework/esm-globals/dist/types.d.ts:90

___

Expand All @@ -36,7 +36,7 @@ URLs of configurations to load in the system.

#### Defined in

packages/framework/esm-globals/dist/types.d.ts:97
packages/framework/esm-globals/dist/types.d.ts:103

___

Expand All @@ -50,7 +50,7 @@ The environment to use.

#### Defined in

packages/framework/esm-globals/dist/types.d.ts:93
packages/framework/esm-globals/dist/types.d.ts:99

___

Expand All @@ -64,7 +64,7 @@ Defines if offline should be supported by installing a service worker.

#### Defined in

packages/framework/esm-globals/dist/types.d.ts:102
packages/framework/esm-globals/dist/types.d.ts:108

___

Expand All @@ -76,4 +76,4 @@ The base path for the SPA root path.

#### Defined in

packages/framework/esm-globals/dist/types.d.ts:88
packages/framework/esm-globals/dist/types.d.ts:94
4 changes: 2 additions & 2 deletions packages/framework/esm-framework/docs/interfaces/WorkspaceGroupDefinition.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ List of workspace names which are part of the workspace group.

#### Defined in

packages/framework/esm-globals/dist/types.d.ts:342
packages/framework/esm-globals/dist/types.d.ts:348

___

Expand All @@ -31,4 +31,4 @@ Name of the workspace group. This is used to launch the workspace group

#### Defined in

packages/framework/esm-globals/dist/types.d.ts:338
packages/framework/esm-globals/dist/types.d.ts:344
6 changes: 6 additions & 0 deletions packages/framework/esm-globals/src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,12 @@ declare global {
* The build number of the app shell. Set when the app shell is built by webpack.
*/
spaVersion: string;
/**
* Globals added by the @single-spa/import-map-injector package.
*/
importMapInjector: {
Copy link

@internettrans internettrans Jun 13, 2025

Choose a reason for hiding this comment

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

This is defined in the types published to npm. Is it possible to reuse those types?

https://app.unpkg.com/@single-spa/import-map-injector@2.0.2/files/types/import-map-injector.d.ts#L11

initPromise: Promise<void>;
};
/**
* Gets a set of options from the import-map-overrides package.
*/
Expand Down
3 changes: 2 additions & 1 deletion packages/shell/esm-app-shell/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,13 @@
"@internationalized/date": "^3.8.0",
"@openmrs/esm-framework": "workspace:*",
"@openmrs/esm-styleguide": "workspace:*",
"@single-spa/import-map-injector": "^2.0.2",
"dayjs": "^1.11.13",
"dexie": "^3.0.3",
"html-webpack-plugin": "^5.5.0",
"i18next": "^21.10.0",
"i18next-browser-languagedetector": "^6.1.8",
"import-map-overrides": "^3.0.0",
"import-map-overrides": "^6.0.1",
"lodash-es": "4.17.21",
"mini-css-extract-plugin": "^2.9.1",
"react": "^18.1.0",
Expand Down
26 changes: 13 additions & 13 deletions packages/shell/esm-app-shell/src/index.ejs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,35 +10,35 @@
href="<%= openmrsFavicon %>"
>
<meta name="viewport" content="width=device-width, initial-scale=1">
<meta name="importmap-type" content="systemjs-importmap">
<meta name="importmap-type" use-injector>
<%= htmlWebpackPlugin.tags.headTags %>
<% if (openmrsImportmapDef) { %>
<script type="systemjs-importmap"><%= openmrsImportmapDef %></script>
<script type="injector-importmap"><%= openmrsImportmapDef %></script>
<% } else if (openmrsImportmapUrl) { %>
<link
rel="preload"
href="<%= openmrsImportmapUrl %>"
as="fetch"
crossorigin="anonymous"
>
<script type="systemjs-importmap" src="<%= openmrsImportmapUrl %>"></script>
<script type="injector-importmap" src="<%= openmrsImportmapUrl %>"></script>
<% } %>
<% if (openmrsCoreImportmap) { %>
<script type="systemjs-importmap"><%= openmrsCoreImportmap %></script>
<script type="injector-importmap"><%= openmrsCoreImportmap %></script>
Copy link

@internettrans internettrans Jun 13, 2025

Choose a reason for hiding this comment

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

why spaces added? For indentation because of ejs? Output html will have indentation bc of this?

Copy link
Member Author

@ibacher ibacher Jun 13, 2025

Choose a reason for hiding this comment

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

Mostly for consistency. It matches the indentation of all the other sub-elements of <head>.

<% } %>
<% if (openmrsRoutesDef) { %>
<script type="openmrs-routes"><%= openmrsRoutesDef %></script>
<script type="openmrs-routes"><%= openmrsRoutesDef %></script>
<% } else if (openmrsRoutesUrl) { %>
<link
rel="preload"
href="<%= openmrsRoutesUrl %>"
as="fetch"
crossorigin="anonymous"
>
<script type="openmrs-routes" src="<%= openmrsRoutesUrl %>"></script>
<link
rel="preload"
href="<%= openmrsRoutesUrl %>"
as="fetch"
crossorigin="anonymous"
>
<script type="openmrs-routes" src="<%= openmrsRoutesUrl %>"></script>
<% } %>
<% if (openmrsCoreRoutes) { %>
<script type="openmrs-routes"><%= openmrsCoreRoutes %></script>
<script type="openmrs-routes"><%= openmrsCoreRoutes %></script>
<% } %>
</head>
<body>
Expand Down
Loading
Loading