fix(toolbox): resolve Vite 8 build warnings in conformance and workbench - #2117
fix(toolbox): resolve Vite 8 build warnings in conformance and workbench#2117MFA-G wants to merge 3 commits into
Conversation
- fdc3-conformance: use `import.meta.dirname` instead of `__dirname` so the config is compatible with `configLoader: 'native'`. - fdc3-conformance: replace `@rollup/plugin-inject` with Rolldown's built-in `build.rollupOptions.transform.inject`, silencing PREFER_BUILTIN_FEATURE and dropping a dependency. - fdc3-conformance: make `fix-source-map-support-global` return a sourcemap via magic-string, silencing SOURCEMAP_BROKEN (the build sets `sourcemap: true`). - fdc3-conformance: pin CSS injection to the `fdc3-compliance` entry, the only entry that imports CSS, instead of letting the plugin pick one of the mocks. - fdc3-workbench: add `"type": "module"` so its ESM `vite.config.ts` is loaded as ESM. Closes finos#2115 for warnings 1, 4, 5 and 7.
|
❌ Deploy Preview for fdc3 failed. Why did it fail? →
|
|
Hi @MFA-G thanks for having a crack at this. Could you take a look at teh Easy CLA comment and hit the Please click here to be authorized link to and complete a CAL so we can merge this? |
kriswest
left a comment
There was a problem hiding this comment.
We could lose magic-string from this and a CLA needs completing before it can be merged.
@julianna-ciq could you review and merge if/when this gets updated?
| "@types/chai": "^4.3.0", | ||
| "@types/mocha": "^10.0.10", | ||
| "http-server": "^14.1.1", | ||
| "magic-string": "^0.30.17", |
There was a problem hiding this comment.
THis is an old verison, current is 1.2.0.
The library is also not wonderfully well maintained (OpenSSF scorecard result is 3.8). Any chance we can eliminate it and use conventional string manipulation without a dependency?
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2117 +/- ##
=======================================
Coverage 95.11% 95.11%
=======================================
Files 73 73
Lines 4196 4196
Branches 584 589 +5
=======================================
Hits 3991 3991
Misses 205 205 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Address review: magic-string was pulled in only to produce a sourcemap for the one-line browser-source-map-support rewrite. Since the replacement is confined to a single line and changes no line numbers, a small hand-rolled identity sourcemap serves the same purpose with no dependency. @rollup/plugin-inject goes with it (replaced by Rolldown's transform.inject in the previous commit), so the package now adds no new devDependency at all.
|
Thanks @kriswest — good call, It was only there to hand back a sourcemap for the function identitySourceMap(code: string, id: string) {
const lineCount = code.split('\n').length;
// VLQ segments: `AAAA` = [col 0, source 0, line 0, col 0]; `AACA` advances the source line by 1.
const mappings = ['AAAA', ...Array(Math.max(lineCount - 1, 0)).fill('AACA')].join(';');
return { version: 3 as const, sources: [id], sourcesContent: [code], names: [], mappings };
}and the transform is back to a simple Net effect on Validation on the updated branch:
The EasyCLA is being sorted out. @julianna-ciq ready for another look whenever convenient. |
|
@MFA-G We'd be happy to move this PR along towards being merged, but can't do so unless you complete the CLA check. See comment #2117 (comment) |
Fix Vite 8 build warnings in
fdc3-conformanceandfdc3-workbenchCloses #2115 (warnings 1, 4, 5 and 7).
After this change
toolbox/fdc3-conformancebuilds completely warning-free, and theconfigLoader: 'native'warning is gone fromtoolbox/fdc3-workbenchtoo.Changes
1.
configLoader: 'native'compatibility (warning #1)fdc3-conformanceused__dirnameinvite.config.ts, which the native config loader does not provide:+const __dirname = import.meta.dirname;import.meta.dirnameis available from Node 20.11, and the package already requires>=22.fdc3-workbenchloaded its ESMvite.config.tsas CommonJS. Added"type": "module"to itspackage.json, as suggested in the issue. The workbench has no.js/.cjssource files that could be affected — all sources are.ts/.tsxcompiled by Vite, andeslint.config.mjsis already explicit.2.
PREFER_BUILTIN_FEATURE—@rollup/plugin-inject(warning #5)Replaced the plugin with Rolldown's native
inject, which lives undertransforminbuild.rollupOptions:The API is aligned with
@rollup/plugin-inject, so the mapping is unchanged.@rollup/plugin-injectis dropped fromdevDependencies.3.
SOURCEMAP_BROKEN—fix-source-map-support-global(warning #4)The build sets
sourcemap: true, so suppressing the warning would have been the wrong trade-off — the plugin really was invalidating the sourcemap forbrowser-source-map-support. It now performs the same single replacement throughmagic-stringand returns the generated map:It also returns early when the pattern is absent instead of silently producing identical output, so a future upstream change to
source-map-supportfails loudly rather than shipping a broken global shim.magic-stringis added as an explicitdevDependency(it was previously only reachable transitively via@rollup/plugin-inject).4. CSS injection entry selection (warning #7)
Only
src/test/index.ts(thefdc3-complianceentry) imports CSS —import 'mocha/mocha.css'. With ~20 entries the plugin was pickingopen-a.jsarbitrarily, meaning the mocha stylesheet was injected by a mock app rather than by the test runner. Now pinned explicitly:Verified in
dist/lib: after the change the mocha CSS and itsdocument.head.appendChildcall appear infdc3-compliance.jsonly, and no longer inopen-a.js.Not addressed
Deliberately left out of this PR, since they are judgement calls rather than fixes:
COMMONJS_VARIABLE_IN_ESM(@jsdevtools/ono) — comes from a transitive dependency's published ESM build (@apidevtools/json-schema-ref-parser→@jsdevtools/ono), not from repo code. Fixing it means patching or replacing a dependency.PLUGIN_TIMINGS(legacy plugin) — both informational, and the issue itself notes 1.0 documentation completion task list #6 is a browser-support policy decision (dropping@vitejs/plugin-legacy) rather than a bug.Happy to follow up on any of those in a separate PR if maintainers have a preference on the direction.
Validation
Run locally on Node 22.23.2 / npm 12.0.2:
npm run build(all workspaces)npm test(all workspaces)npm run lint(all workspaces)npx prettier --checkon changed filesnpx depcheck --skip-missingin both toolbox workspacesvite buildwarnings infdc3-conformancevite buildwarnings infdc3-workbenchconfigLoaderwarning gone; remaining are #2/#3/#6 aboveCHANGELOG.mdupdated under[Unreleased] → Fixed.