Repository navigation
fix(builder): keep federation externals out of the dev server's stylesheet build - #169
JasinRefiku wants to merge 3 commits into
Conversation
…sheet build The dev server added every federation external to Angular's externalDependencies so that serveWithVite routes them to Vite's optimizeDeps.exclude. @angular/build also passes that option to its stylesheet bundler, and esbuild treats an external package name as covering its subpaths, so a Sass `@use '@taiga-ui/core/styles/...'` of a shared package failed with "Can't find stylesheet to import". `ng build` was unaffected. serveWithVite reads externalDependencies back from the options object it passed in, so the full list stays there for Vite. The build itself now gets the target's own list when the `externals` plugin already marks the browser bundle's externals and there is no server bundle. With a server bundle, or with the plugin off for a federated SSR build, the build keeps them: server bundles and their polyfills don't run the plugin, and Angular only adds the ngServerMode banner for externals when externalDependencies is set. Fixes native-federation#167
Aukevanoost
left a comment
There was a problem hiding this comment.
Thanks for tracking this down, the stylesheet bundler test is a nice touch! A few things I noticed below.
| // ngServerMode banner Angular adds for externals only get them from here. | ||
| const buildOptions = | ||
| opts?.externalsPlugin && !options.server | ||
| ? { ...options, externalDependencies: targetExternals } |
There was a problem hiding this comment.
Since normalizeOptions mutates the options it receives (options.ssr = false / options.prerender = !!options.server when outputMode is set without server), those corrections now land on the copy and serveWithVite keeps reading the original browserOptions.ssr. So e.g. outputMode: 'static', ssr: true without server would pick InternalSsrMiddleware (dev-server/vite/index.js) while no server bundle is built. Could we carry ssr back, or avoid the copy for that case?
There was a problem hiding this comment.
Fixed in 83cac6c. The build now gets a Proxy over the served options that only overrides the externalDependencies read, so normalizeOptions' ssr/prerender corrections land on the object serveWithVite reads. Added a spec for outputMode + ssr without server.
| const buildOptions = | ||
| opts?.externalsPlugin && !options.server | ||
| ? { ...options, externalDependencies: targetExternals } | ||
| : options; |
There was a problem hiding this comment.
One more on the copy: the browser polyfills bundle doesn't get the codePlugins, and Angular disables externalPackages for it as soon as a polyfill is a local file (getEsBuildCommonPolyfillsOptions). So externalDependencies was the only thing keeping shared packages external there; with this change a polyfills.ts importing a shared package (e.g. @angular/localize/init under shareAll) would inline its own copy. Maybe keep the federation externals for the polyfills somehow, or limit the change to the stylesheet side?
There was a problem hiding this comment.
Applied in 2575c0a, as a separate commit: when any polyfill is a local file (same check as Angular's isLocalFile), the build keeps the federation externals, so a polyfills.ts behaves as before.
One pushback, though: ng build already inlines shared packages imported from a local polyfills file. For CSR the builder only passes the federation externals through the externals plugin (externalDependencies is only set in the activateSsr branch of builder.ts), and the plugin doesn't reach the polyfills bundle. So without 2575c0a the dev server matches production and apps with a polyfills.ts get the #167 fix too. With it, those apps still hit #167 under ng serve. (@angular/localize/init is in NG_SKIP_LIST, so it isn't shared under shareAll.)
I'm fine either way. If you'd rather have dev/prod parity, I'll revert 2575c0a.
| createInternalAngularBuilder(externals, { | ||
| instrumentForCoverage: nfBuilderOptions.instrumentForCoverage, | ||
| define: nfBuilderOptions.define, | ||
| externalsPlugin: !activateSsr, |
There was a problem hiding this comment.
Small note: with activateSsr the full list still goes into ngBuilderOptions.externalDependencies (line 319), so federated SSR prod builds (and the server case in the internal builder) still hit #167. Fine to leave for a follow-up, but might be worth mentioning in the PR. Also, this flag duplicates the condition in the externals plugin (line 301); deriving both from one variable would keep them in sync.
There was a problem hiding this comment.
Both SSR cases (SSR dev serving with a server entry, and federated SSR production builds) are listed under "Not covered" in the description, left for a follow-up. The duplicated condition is fixed in 83cac6c: useExternalsPlugin now drives both the plugin and the externalsPlugin option.
normalizeOptions writes its `ssr` and `prerender` corrections onto the options it gets (when `outputMode` is set without `server`), and serveWithVite picks its SSR middleware from the options it passed in. With the shallow copy those writes landed on the copy, so `outputMode` with `ssr` and no `server` chose InternalSsrMiddleware without a server bundle. The build now gets a proxy that only overrides the externalDependencies read, so the writes reach the original options. Also derive the `externals` plugin condition and the internal builder's `externalsPlugin` option from one variable.
…ill is a local file Angular turns off `externalPackages` for the polyfills bundle as soon as a polyfill is a local file, and code plugins don't reach that bundle, so externalDependencies was all that kept a shared package imported from `polyfills.ts` external under the dev server. Keep the federation externals in the build in that case (using a copy of Angular's isLocalFile), so those polyfills behave as before. Such apps still hit native-federation#167 under the dev server. `ng build` already inlines shared packages imported from a local polyfills file, since its externals only come from the `externals` plugin. Reverting this commit makes the dev server match it and fixes native-federation#167 for those apps too.
Fixes #167.
Problem
Under the dev server,
createInternalAngularBuilderadded every federation external to Angular'sexternalDependenciesso thatserveWithViteroutes them to Vite'soptimizeDeps.exclude. But@angular/build(22.2.1) passes that same normalized option to its stylesheet bundles too:builders/application/setup-bundling.js,createComponentStyleBundler:externalDependenciesgoes toComponentStylesheetBundler.tools/esbuild/global-styles.js,createGlobalStylesBundleOptions: the same for global styles.tools/esbuild/stylesheets/bundle-options.js:external: options.externalDependencies.esbuild treats an external package name as covering its subpaths. So when the Sass importer in
tools/esbuild/stylesheets/sass-language.jscallsbuild.resolve('@taiga-ui/core/styles/mixins/mixins', { kind: 'import-rule' }), it gets an external result with no path. Its<pkg>/package.jsonfallback is external too, and Sass reports "Can't find stylesheet to import".ng buildisn't affected, because there the externals only reach the browser bundle, through theexternalsplugin.Where the externals actually need to go
I traced
externalDependenciesthrough@angular/build22.2.1:builders/dev-server/vite/index.jspassesbrowserOptionsto the builder action and, after each result, callsupdateExternalMetadata(result, externalMetadata, browserOptions.externalDependencies). It reads the list back from the object it passed in.tools/vite/utils.jsadds that list toexplicitBrowserandexplicitServer, anddev-server/vite/server.jsturns those intooptimizeDeps.exclude,ssr.external/ssr.optimizeDeps.exclude, the memory plugin's externals and the/@id/prefix removal. The full list matters here, not just what the app imports directly. A shared package imported only from inside a non-shared package that Vite pre-bundles must still be excluded, or the pre-bundle inlines a second copy.externalsplugin setsbuild.initialOptions.externalon every non-nodecode bundle unlessactivateSsris set, and that replaces whatever came fromexternalDependencies. esbuild'sinitialOptionsis the options object itself, sobundler-context.jsalso reports these externals asexternalConfiguration, andexecute-build.jsclassifies matching imports as explicit externals.ngServerModebanner. Code plugins only run in the browser main, server main and SSR entry bundles (application-code-bundle.js). The browser and server polyfill bundles takeexternalfromexternalDependenciesonly. Angular also addsglobalThis['ngServerMode'] = trueto the SSR entry and server polyfills only whenexternalDependencies(or the polyfills'external) is non-empty (fix(@angular/build): handle external@angular/packages during SSR angular/angular-cli#29094). The builder's plugin skipsplatform: 'node'.Change
serveWithVitepassed in still gets the full list (target externals plus federation externals), so Vite behaves exactly as before.Proxyover those same options whoseexternalDependenciesis the target's own list, but only when all of these hold:externalsplugin covers the browser bundle (newexternalsPluginoption; the builder derives it and the plugin's condition from one variable),options.serverunset; Angular only builds server bundles from aserverentry), andisLocalFilecheck; see below).normalizeOptionsinbuilders/application/options.jssetsssr = falseandprerender = !!serverwhenoutputModeis set, andserveWithVitepicks its SSR middleware fromssrafterwards. A shallow copy would swallow those corrections.What changes for a CSR dev server:
externalsplugin overridesexternalanyway.prebundleon (the default) the output is the same: every package is external there, and the mergedexternalConfigurationfrom the main bundle still classifies shared imports as explicit. Withprebundle: false, a shared package listed inpolyfillsis bundled, asng buildalready does.zone.jsand@angular/localizeare on the skip list, so this rarely applies.Local polyfill files
Angular turns off
externalPackagesfor the polyfills bundle as soon as a polyfill is a local file (getEsBuildCommonPolyfillsOptions), and code plugins don't reach that bundle. SoexternalDependenciesis the only thing keeping a shared package imported frompolyfills.tsexternal, and the build keeps the federation externals in that case. Those apps still hit #167 under the dev server.ng buildalready inlines such imports: for CSR the federation externals only come from theexternalsplugin. Reverting 2575c0a would make the dev server match production and fix #167 for these apps too. It's a separate commit so it can be dropped if parity is preferred.Alternatives considered
ngServerModebanner while Vite still loads the shared Angular packages natively throughssr.external. Withssrand nodev, the browser bundle would lose its externals altogether, because the plugin is off.externalDependencieseverywhere. Code plugins don't reach the polyfill bundles or the banner logic, so this has the same SSR gaps.ApplicationBuilderExtensionsonly hascodePluginsandindexHtmlTransformer.serveWithViteonly takesmiddlewareandbuildPlugins. Stylesheet bundles are created internally (createComponentStyleBundler,createGlobalStylesBundleOptions) from the normalized options, so there's nothing to hook into without deep imports.externalhas no exact-only form for package paths:fooalways impliesfoo/*, and*patterns are prefix/suffix matches. Deep imports would also behave differently.node_modulestostylePreprocessorOptions.includePaths. That's the workaround from the issue. It only helps Sass with hoisted packages, ignores packageexports/styleconditions, and doesn't fix CSS or Less@import.This only changes which options we pass to
buildApplicationInternalandserveWithVite. There are no new imports from@angular/buildand nothing written tonode_modules. The only copied code is Angular's four-lineisLocalFile, tagged// upstream:.Not covered
serverentry) keeps the federation externals in the build, so a stylesheet@useof a shared package still fails there. Fixing it needs either an Angular seam for stylesheet externals or our own handling of the server polyfills andngServerModebanner. TheincludePathsworkaround from the issue still works.ssr: true,dev: false) setngBuilderOptions.externalDependencies = externalsinbuilder.tsand have the same stylesheet issue. Left out to keep this PR focused.Testing
internal-angular-builder.spec.ts:defineandinstrumentForCoveragestill reach both objects.ssr/prerendercorrections written to the build options reach the served options..ts,.mjs,./-relative), the build keeps the federation externals.zone.jsand other package polyfills don't count as local.ComponentStylesheetBundler(from@angular/build/private) compiles@use 'shared-ui/styles/mixins'from a fake package in a temp workspace, using theexternalDependenciesthe build now gets. As a control, the full list reproduces "Can't find stylesheet to import". It takes about 100 ms. Each bundle uses its own workspace root because Angular caches Sass package resolutions per working directory for the whole process.main. The guard tests (server entry, plugin off, local polyfill, option corrections) pass on both. The option-corrections test failed on 164f55a, before theProxy.pnpm lint(no errors; 34 existing warnings, the same as onmain),pnpm typecheck,pnpm buildandknippass.pnpm test: 402 tests pass. On macOS with the defaultTMPDIR,sourcemap-ignorelist-plugin.spec.ts› "marks node_modules sources of a bundle emitted into the federation cache" fails onmaintoo. It compares paths under/var/folders/...with esbuild's/private/var/...realpaths. With a non-symlinkedTMPDIRthe whole suite passes.@taiga-ui/coreas a singleton (Angular 22.2.0, adapter 22.2.1 with this change applied to the installed builder, theincludePathsworkaround removed):nx servefails with "Can't find stylesheet to import" on the@taiga-ui/core/styles/...@uses.remoteEntry.jsonstill lists the shared packages, and Vite'sdepscache holds none of them.