perf: allow tree-shaking for built-in metadata managers

[davidlj95]

After findings in #891 , another thing that isn't tree shakeable right now are metadata managers.

Given the metadata manager constants are defined by calling a helper function to create the provider. So when importing anything from a built-in module, all constants exported are created by calling those helper functions. And given the bundler doesn't know if those calls have side-effects, they are not removed from the bundle even if unused.

Implementing tree-shakeable ones is as easy as defining a function instead of a const. Given until the function isn't called, no side-effects may happen so the bundler safely removes those from bundle if unused.

In few words:

// `const`-based, not tree-shakeable as calling `provide...` may have side effects
// So the `const` ends up in the bundle even if unused
export const OPEN_GRAPH_TITLE_METADATA_PROVIDER = provideOpenGraphManager(...)

// into

// Function-based, tree-shakeable. As until not used (called), no side-effects happen for sure
// Bundlers can safely remove if unused.
export const provideOpenGraphTitle = () => provideOpenGraphManager(...)

The issue is that if just adding the new function-based tree-shakeable providers, existing ones will be kept in the bundle even if unused as they're not tree-shakeable. So in order to allow tree-shaking, old const ones must be removed.

Warning

This change implies bundle size increase though

After quickly trying, here's the extra amount of uncompressed bytes added to the bundle if:

• Adding function providers, not removing const ones, not using them for provideX: 4 bytes x 26 managers = 104 bytes
• Adding function providers, not removing const ones, use them for provideX: 10 bytes x 26 managers = 260 bytes
• Adding function providers, removing const ones, use function ones for provideX: 6 bytes x 26 managers = 156 bytes

Removing old const ones is a breaking change 🤔

It's a beta version, so APIs are subject to change. But even in that scenario, I'd prefer to avoid any kind of breaking change. Or at least provide an auto-magic migration / schematic if can't be avoided like it seems the case.

Indeed adding schematics would help in other places such as deprecations

UPDATE: finally can do this without introducing a breaking change. TL;DR: pure annotations. See below 👇

[davidlj95]

Just reminded that maybe annotating as pure could work. And it does indeed. See here an example to mark as pure the Open Graph image metadata provider:

/**
 * Manages the {@link OpenGraph.image} metadata
 * @public
 */
export const OPEN_GRAPH_IMAGE_METADATA_PROVIDER =
  /* @__PURE__ */ provideOpenGraphManager(
    _GLOBAL_IMAGE,
    /* @__PURE__ */ _withModuleManagerSameGlobalKey(),
    /* @__PURE__ */ withManagerObjectMerging(),
    /* @__PURE__ */ _withModuleManagerSetterFactory(
      (metaElementsService: NgxMetaElementsService) => (value) => {
        const imageUrl = value?.url?.toString()
        const effectiveValue: OpenGraph[typeof _GLOBAL_IMAGE] = _isDefined(
          imageUrl,
        )
        // ...
      }
    )
  )

This works in v18 with ESBuild via the application builder and in v16 with Webpack via the browser builder

Argument calls must be also be commented as explained in ESBuild docs:

It's worth mentioning that the effect of the annotation only extends to the call itself, not to the arguments. Arguments with side effects are still kept even when minification is enabled:

echo 'document.createElement(elemName())' | esbuild --pure:document.createElement
/* @__PURE__ */ document.createElement(elemName());
echo 'document.createElement(elemName())' | esbuild --pure:document.createElement --minify
elemName();

[davidlj95]

Hence with this in mind, possible options change:

• 1. Leave const providers (not deprecated), add pure annotations around: essentially applying pure annotations like the example one around
  • PRO: No breaking changes
  • PRO: No bundle size increase
  • CON: Many annotations to add as argument calls must be annotated too
    • FIX: Testing tree-shaking would be great, but would imply yet more infra
  • CON: Easy to miss adding those pure annotations for new providers to come
• 2. Leave const providers (as deprecated), add function providers. Use function provider for const providers with a pure annotation.
  • PRO: No breaking changes
  • CON: Bundle size increase: [156, 260] bytes. When no consts are used, 156 bytes if using all providers. As it's like const ones are removed when unused. Can be up to 260 bytes if consts are used.
  • PRO: Pure annotations just added to deprecated consts, hence...
  • PRO: No more pure annotations for new providers
• 3. Remove const providers, add function providers
  • CON: Breaking change
    • FIX: Schematics to automatically change
  • CON: Bundle size increase. 156 bytes if using all providers.
  • PRO: No pure annotations in any place
  • PRO: No pure annotations for new providers

First, skipping first option and more options implying keeping const providers as:

• Easy to be not tree-shaken by mistake: due to argument calls needing pure annotations too. So they don't get tree-shaken.
• Flexibility: a function provider allows to customize a provider behaviour by introducing arguments to that function. Or to expand its features in a tree-shakeable manner. Like passing an array of features as done in provideNgxMetaCore(withX(), withY(), ...).

Better to deprecate them for those two reasons. And use functions by default from now on.

This leaves us with options 2 and 3. And after balancing pros and cons, seems option 2 is the best one. Breaking changes are avoided, but the benefits of functions are introduced anyway. At some point, const providers can be removed. Hence we can get rid of all pure annotations around.