Skip to content

Commit

Permalink
build: fix unresolved dependencies
Browse files Browse the repository at this point in the history
We don't want to include our dependencies (react, maplibre, and so
on) in the final bundle: because library users download these as
transitive deps we want to leave it up to them to resolve the
imports and include them in the final app bundle. For instance,
OSRD is responsible for inlining maplibre imports in ui-warped-map's
bundle.

On the other hand, relative imports local to the package should be
bundled.

By default, rollup only resolves relative imports, and doesn't
resolve global imports. rollup prints an error when it encounters
a global import that it can't resolve. We don't want to just
ignore unresolved global imports, because that would silently
allow broken non-existing imports.

The @rollup/plugin-node-resolve plugin allows rollup to resolve
global imports like node does. However, by default it includes
all global imports in the bundle. This isn't what we want: we
want to exclude dependencies from the bundle.

To fix this, follow the recommendations in the rollup docs [1]
and use the "external" option to check whether an import points
to the "node_modules" directory or a "dist" directory of any
osrd-ui package. In that case, ask rollup to not bundle the
import.

We don't need to manually specify a list of external imports
from each package anymore.

[1]: https://rollupjs.org/configuration-options/#external

Signed-off-by: Simon Ser <[email protected]>
  • Loading branch information
emersion committed Jan 15, 2025
1 parent 1f83c5f commit 6dd85a7
Show file tree
Hide file tree
Showing 9 changed files with 71 additions and 8 deletions.
50 changes: 50 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
"type": "module",
"devDependencies": {
"@rollup/plugin-eslint": "^9.0.5",
"@rollup/plugin-node-resolve": "^16.0.0",
"@rollup/plugin-terser": "^0.4.4",
"@rollup/plugin-typescript": "^12.1.0",
"@types/lodash": "^4.17.13",
Expand Down
13 changes: 11 additions & 2 deletions rollup-base.config.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,18 @@
import process from 'process';
import path from 'path';

import eslint from '@rollup/plugin-eslint';
import nodeResolve from '@rollup/plugin-node-resolve';
import terser from '@rollup/plugin-terser';
import typescript from '@rollup/plugin-typescript';
import livereload from 'rollup-plugin-livereload';
import postcss from 'rollup-plugin-postcss';

const formats = ['esm'];
const rootDir = path.join(process.cwd(), '..');

/** @type {import("rollup").RollupOptions} */
const generateRollupBaseConfig = (projectName, external) => ({
const generateRollupBaseConfig = (projectName) => ({
input: 'src/index.ts',
output: formats.map((format) => ({
file: `dist/index.${format}.js`,
Expand All @@ -18,6 +21,7 @@ const generateRollupBaseConfig = (projectName, external) => ({
sourcemap: true,
})),
plugins: [
nodeResolve({ rootDir }),
eslint(),
typescript(),
postcss({
Expand All @@ -31,7 +35,12 @@ const generateRollupBaseConfig = (projectName, external) => ({
watch: 'dist',
}),
],
external,
external: (id, parent, isResolved) => {
if (!isResolved) return false;
const rel = path.relative(rootDir, id);
const filenames = rel.split(path.sep);
return filenames[0] === 'node_modules' || filenames[1] === 'dist';
},
});

export default generateRollupBaseConfig;
3 changes: 2 additions & 1 deletion ui-core/rollup.config.js
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
import generateBaseRollupConfig from '../rollup-base.config.js';
export default generateBaseRollupConfig('osrdcore', ['react']);

export default generateBaseRollupConfig('osrdcore');
3 changes: 2 additions & 1 deletion ui-manchette-with-spacetimechart/rollup.config.js
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
import generateBaseRollupConfig from '../rollup-base.config.js';
export default generateBaseRollupConfig('osrdcore', ['react']);

export default generateBaseRollupConfig('osrdcore');
3 changes: 2 additions & 1 deletion ui-manchette/rollup.config.js
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
import generateBaseRollupConfig from '../rollup-base.config.js';
export default generateBaseRollupConfig('osrdcore', ['react']);

export default generateBaseRollupConfig('osrdcore');
2 changes: 1 addition & 1 deletion ui-spacetimechart/rollup.config.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
import generateBaseRollupConfig from '../rollup-base.config.js';

export default generateBaseRollupConfig('osrdcore', ['react']);
export default generateBaseRollupConfig('osrdcore');
2 changes: 1 addition & 1 deletion ui-speedspacechart/rollup.config.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
import generateBaseRollupConfig from '../rollup-base.config.js';

export default generateBaseRollupConfig('osrdcore', ['react']);
export default generateBaseRollupConfig('osrdcore');
2 changes: 1 addition & 1 deletion ui-trackoccupancydiagram/rollup.config.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
import generateBaseRollupConfig from '../rollup-base.config.js';

export default generateBaseRollupConfig('osrdTrackOccupancyDiagram', ['react']);
export default generateBaseRollupConfig('osrdTrackOccupancyDiagram');

0 comments on commit 6dd85a7

Please sign in to comment.