Allow v2 addons to filter app-js

[jelhan]

Some addons provided "poor-man's" tree-shaking (© @simonihmig) for classic Ember apps to overcome the limitations of Ember's classic build system. They allow the consumer to define an include / exclude list to filter, which components, helpers, and modifiers are pushed into the consuming app's JavaScript.

I'm aware of the following addons having such a feature:

• ember-bootstrap^[1]
• ember-math-helpers^[2]
• ember-composable-helpers

Please note that the first two, ember-bootstrap and ember-math-helpers, are listed as top 25 addons by Ember Observer.

The "poor-man's" tree-shaking is implemented using the treeForApp hook of v1 addons. The tree is filtered based on user's configuration.

v2 addons does not provide a similar capability yet. This forces such addons to either hold back migration to v2 or drop the "poor-man's" tree-shaking capability. Dropping the capability to tree-shake components, helpers, and modifiers pushed into the consumers JavaScript bundle would be a regression for all consumers not ready to use Embroider with a { staticComponents: true, staticHelpers: true, staticModifiers: true } configuration. This is forcing a challenging trade-off decision on maintainers of such addons.

It would be great if v2 addons support filtering the app-js based on configuration provided by the consumer. Please note that it is only about excluding files listed in app-js. It is not about pushing additional files into the app-js.

For non-Embroider apps, it should be rather simple to implement. @embroider/addon-shim already excepts options. Additionally to the existing disabled option, it could accept a filterAppJs function, which receives a file name as input and returns a boolean. If returning false, the file is filtered out. The function would be called in the treeForApp hook of the addon shim:

embroider/packages/addon-shim/src/index.ts

Lines 69 to 92 in 7357b5a

	treeForApp(this: AddonInstance) {
	if (disabled) {
	return undefined;
	}
	let maybeAppJS = meta['app-js'];
	if (maybeAppJS) {
	const appJS = maybeAppJS;
	return buildFunnel(rootTree(this), {
	files: Object.values(appJS),
	getDestinationPath(relativePath: string): string {
	for (let [exteriorName, interiorName] of Object.entries(appJS)) {
	if (relativePath === interiorName) {
	return exteriorName;
	}
	}
	throw new Error(
	`bug in addonV1Shim, no match for ${relativePath} in ${JSON.stringify(
	appJS
	)}`
	);
	},
	});
	}
	},

I'm not sure how it could be implemented for Embroider apps. I found similar code reading the ember-addon.app-js from package.json:

embroider/packages/core/src/app-files.ts

Lines 48 to 55 in 7357b5a

	let appJS = addon.meta['app-js'];
	if (appJS) {
	for (let filename of Object.keys(appJS)) {
	filename = filename.replace(/^\.\//, '');
	combinedFiles.add(filename);
	combinedNonFastbootFiles.add(filename);
	}
	}

I assume filtering could be done at that step. But I'm not sure how the filtering function should be provided for v2 addons. And if such a capability would fit into the overall architecture of Embroider.

[1] v2 addon migration for Ember Bootstrap is discussed here: ember-bootstrap/ember-bootstrap#1980
[2] Filtering based on only / except configuration is broken in latest ember-math-helpers release. It seem that this functionality was not considered when the addon got migrated to a v2 addon. See RobbieTheWagner/ember-math-helpers#1323 for details.

[ef4]

I don't really want to do this. It's an extra feature to get a small optimization that only matters in our least-optimized use case, and it contradicts one of the most important reasons v2 addons exists (their code can be known without running any custom code first).

There's nothing to stop you from extending the return value of addonV1Shim to take control over what happens in classic builds. I normally discourage that because it won't have any effect in embroider builds. But in this case, it's arguably classic builds who need the filtering more anyway, and you could tell people "only doesn't have any effect on embroider, there you should use staticComponents to get the same effect".

An alternative that isn't perhaps as radical as it sounds is to stop putting components, helpers, or modifiers into app-js entirely. I think it already makes sense to start migrating addon documentation to explain how they're supposed to be used via template tag, and explain the non-template-tag case explicitly:

// Using template tag, you import the component like this:
import BsAccordion from 'ember-bootstrap/components/bs-accordion';
<template>
  <BsAccordion />
</tempate>

// If you're not using template tag, you need to make a separate file to do the import.
// Create app/components/bs-accordion.js containing:
export { default } from 'ember-bootstrap/components/bs-accordion';

This gives full control over tree-shaking to both embroider and non-embroider apps, because the actual ember-bootstrap/ namespace gets tree-shaking even in classic builds if ember-bootstrap is a v2 addon, and the app can choose to delete any app-js reexports they're not using.

Tangentially: if you're going to do a semver major anyway, it makes sense for addon authors to streamline the naming of their components and own JS modules (as opposed to app-js modules) so that they're nicer to use with template tag. There's no need for prefixes on all the names once people aren't accessing the names from one global pool.

-import BsAccordion from 'ember-bootstrap/components/bs-accordion';
+import Accordion from 'ember-bootstrap/accordion';