RC #291#292
Merged
Merged
Conversation
Demo entries that imported common modules caused Rollup to emit shared chunks like index2.min.js, breaking demo HTML which only loads <name>.min.js. Each demo entry now builds as its own Rollup pass with inlineDynamicImports so shared imports duplicate into each <name>.min.js instead of spilling into a separate chunk. Also bumps @actions/github to ^9.1.1 and @rollup/plugin-terser to ^1.0.0, and updates github-labels.ts to the new namespace import shape.
Demo entries importing workspace packages (e.g. @aurodesignsystem/auro-datepicker) were sometimes left as bare specifiers in the output, causing the browser to throw "Failed to resolve module specifier" at runtime. nodeResolve's auto-walkup didn't reach the hoist root in some monorepo / symlinked workspace setups. Pass MODULE_DIRS as absolute modulePaths to nodeResolve so it falls back to the same parent node_modules chain the SCSS importer already uses, ensuring hoisted deps get inlined into each <name>.min.js.
Convert Rollup's UNRESOLVED_IMPORT warning into a hard build error. Previously a missing workspace dep (e.g. a sibling component not yet built) was silently externalized, leaving a bare specifier in the demo .min.js that the browser rejected at runtime. Now the build fails loudly with a pointer to fix the build order.
Reviewer's GuideRefactors the demo Rollup build pipeline to emit one bundle per demo entry with inline dynamic imports, tightens unresolved import handling, adjusts watch/combined build wiring to match, introduces absolute module resolution fallbacks, and bumps a couple of build-related dependencies plus a small TypeScript typing fix. Sequence diagram for updated demo Rollup build and unresolved import handlingsequenceDiagram
actor Dev
participant index as runProductionBuild
participant demo as buildDemoBundle
participant config as getDemoConfig
participant rb as rollup
participant handler as runBuildStep
participant cfg as demo_entry_config
participant defaultHandler
Dev->>index: runProductionBuild(options)
index->>config: getDemoConfig(options)
config-->>index: { name, configs }
index->>demo: buildDemoBundle({ configs })
demo->>handler: runBuildStep("Bundling demo JS...")
loop for each cfg in configs
handler->>rb: rollup(cfg)
rb-->>handler: bundle
handler->>rb: write(cfg.output)
handler->>rb: close()
end
alt [warning.code === UNRESOLVED_IMPORT]
rb->>cfg: onwarn(warning, defaultHandler)
cfg->>cfg: throw Error
else [other warnings]
rb->>cfg: onwarn(warning, defaultHandler)
cfg->>defaultHandler: defaultHandler(warning)
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
jason-capsule42
approved these changes
May 11, 2026
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- In
getPluginsConfig,allModulePathsis now completely unused; consider either wiring it intonodeResolvealongsideabsoluteModulePathsor removing it to avoid dead code and confusion about which module path sources are honored. - The logic that iterates over demo Rollup configs and builds them one-by-one is duplicated in
buildCombinedBundleandbuildDemoBundle; consider extracting this into a shared helper to keep behavior consistent and reduce future drift.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `getPluginsConfig`, `allModulePaths` is now completely unused; consider either wiring it into `nodeResolve` alongside `absoluteModulePaths` or removing it to avoid dead code and confusion about which module path sources are honored.
- The logic that iterates over demo Rollup configs and builds them one-by-one is duplicated in `buildCombinedBundle` and `buildDemoBundle`; consider extracting this into a shared helper to keep behavior consistent and reduce future drift.
## Individual Comments
### Comment 1
<location path="src/scripts/build/configUtils.js" line_range="36-43" />
<code_context>
+ // when the importer's auto-walkup chain doesn't reach the hoist root
+ // (e.g. CLI invoked from a sibling repo, symlinked workspaces).
+ const cwd = process.cwd();
+ const absoluteModulePaths = MODULE_DIRS.map((dir) => resolve(cwd, dir));
+
return [
nodeResolve({
dedupe,
preferBuiltins: false,
moduleDirectories: DEFAULTS.moduleDirectories,
+ modulePaths: absoluteModulePaths,
}),
commonjs(),
</code_context>
<issue_to_address>
**issue (bug_risk):** User-provided module paths and DEFAULTS.modulePaths appear to be ignored by nodeResolve now.
`allModulePaths = [...DEFAULTS.modulePaths, ...modulePaths]` is still computed but unused, and `nodeResolve` now only receives `absoluteModulePaths` from `MODULE_DIRS`. As a result, any `modulePaths` passed into `getPluginsConfig` (and possibly `DEFAULTS.modulePaths`) are ignored. If the goal is to add absolute fallbacks rather than change behavior, please ensure these paths are included, e.g. by merging them into `modulePaths: [...absoluteModulePaths, ...allModulePaths]` (with deduping as needed) or folding `allModulePaths` into `absoluteModulePaths`.
</issue_to_address>
### Comment 2
<location path="src/scripts/build/configUtils.js" line_range="136-144" />
<code_context>
+ // Rollup silently externalize it — a bare specifier in a demo .min.js
+ // breaks at runtime in the browser. Most common cause: a workspace
+ // dep wasn't built yet (fix the build order, e.g. turbo `^build`).
+ onwarn(warning, defaultHandler) {
+ if (warning.code === "UNRESOLVED_IMPORT") {
+ throw new Error(
+ `Unresolved import "${warning.exporter ?? warning.source}" in ${warning.id ?? file}. ` +
+ "Make sure workspace dependencies are built before bundling demos.",
+ );
+ }
+ defaultHandler(warning);
+ },
+ watch: watcher,
</code_context>
<issue_to_address>
**suggestion:** Consider preserving the original Rollup warning message in the thrown error for better diagnostics.
The thrown `Error` currently only includes `warning.exporter ?? warning.source` and `warning.id ?? file`, but not `warning.message`. This reduces the diagnostic detail Rollup provides (e.g., exact specifier and search paths) and can make UNRESOLVED_IMPORT issues harder to debug. Consider including `warning.message` in the error (or serializing the full `warning` via `JSON.stringify`) so consumers have the full context while still failing the build.
```suggestion
onwarn(warning, defaultHandler) {
if (warning.code === "UNRESOLVED_IMPORT") {
throw new Error(
`Unresolved import "${warning.exporter ?? warning.source}" in ${warning.id ?? file}. ` +
"Make sure workspace dependencies are built before bundling demos. " +
`(Rollup: ${warning.message})`,
);
}
defaultHandler(warning);
},
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Member
|
🎉 This PR is included in version 3.7.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
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.
Alaska Airlines Pull Request
Release candidate pull request. See issue #291 for details.
Checklist:
By submitting this Pull Request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
Pull Requests will be evaluated by their quality of update and whether it is consistent with the goals and values of this project. Any submission is to be considered a conversation between the submitter and the maintainers of this project and may require changes to your submission.
Thank you for your submission!
-- Auro Design System Team
Summary by Sourcery
Adjust demo bundling and module resolution to ensure each demo entry produces a self-contained bundle and improve reliability of workspace builds.
Enhancements:
.min.jsis self-contained and eliminating extra shared chunk files.Build:
@actions/githuband@rollup/plugin-terserdependency versions inpackage.jsonand lockfile to keep build tooling up to date.