Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate staticHelpers, staticModifiers, and staticComponents in favour of staticInvoakables #2210

Open
wants to merge 4 commits into
base: stable
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 3 additions & 6 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -82,9 +82,7 @@ You can pass options into Embroider by passing them into the `compatBuild` funct
return require('@embroider/compat').compatBuild(app, Webpack, {
// staticAddonTestSupportTrees: true,
// staticAddonTrees: true,
// staticHelpers: true,
// staticModifiers: true,
// staticComponents: true,
// staticInvokables: true,
// staticEmberSource: true,
// splitAtRoutes: ['route.name'], // can also be a RegExp
// packagerOptions: {
Expand All @@ -99,9 +97,8 @@ The recommended steps when introducing Embroider into an existing app are:

1. First make it work with no options. This is the mode that supports maximum backward compatibility. If you're hitting errors, first look at the "Compatibility with Classic Builds" section below.
2. Enable `staticAddonTestSupportTrees` and `staticAddonTrees` and test your application. This is usually safe, because most code in these trees gets consumed via `import` statements that we can analyze. But you might find exceptional cases where some code is doing a more dynamic thing.
3. Enable `staticHelpers` and `staticModifiers` and test. This is usually safe because addon helpers and modifiers get invoked declaratively in templates and we can see all invocations.
4. Enable `staticComponents`, and work to eliminate any resulting build warnings about dynamic component invocation. You may need to add `packageRules` that declare where invocations like `{{component someComponent}}` are getting `someComponent` from.
5. Once your app is working with all of the above, you can enable `splitAtRoutes` and add the `@embroider/router` and code splitting should work. See the packages/router/README.md for details and limitations.
3. Enable `staticInvokables` and work to eliminate any resulting build warnings about dynamic component invocation. You may need to add `packageRules` that declare where invocations like `{{component someComponent}}` are getting `someComponent` from.
4. Once your app is working with all of the above, you can enable `splitAtRoutes` and add the `@embroider/router` and code splitting should work. See the packages/router/README.md for details and limitations.

## Configuring asset URLs

Expand Down
59 changes: 45 additions & 14 deletions packages/compat/src/compat-app-builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,24 +60,60 @@ import escapeRegExp from 'escape-string-regexp';

import type CompatApp from './compat-app';
import { SyncDir } from './sync-dir';
import type { CompatOptionsType } from './options';

// This exists during the actual broccoli build step. As opposed to CompatApp,
// which also exists during pipeline-construction time.

export class CompatAppBuilder {
// for each relativePath, an Asset we have already emitted
private assets: Map<string, InternalAsset> = new Map();
private staticComponents = false;
private staticHelpers = false;
private staticModifiers = false;

constructor(
private root: string,
private origAppPackage: Package,
private appPackageWithMovedDeps: Package,
private options: Required<Options>,
private options: CompatOptionsType,
private compatApp: CompatApp,
private configTree: V1Config,
private synthVendor: Package,
private synthStyles: Package
) {}
) {
// staticInvokables always wins when configured
if (typeof options.staticInvokables !== 'undefined') {
if (
typeof options.staticComponents !== 'undefined' ||
typeof options.staticHelpers !== 'undefined' ||
typeof options.staticModifiers !== 'undefined'
) {
throw new Error(
'You cannot set `staticHelpers`, `staticComponents`, or `staticModifiers` if you have set `staticInvokables`. Delete these configs to continue.'
);
}
this.staticComponents = this.staticHelpers = this.staticModifiers = options.staticInvokables;
return;
}

if (typeof options.staticComponents !== 'undefined') {
// TODO it doesn't seem like we have any real deprecation functionality in this package yet.
// do we need it?
console.error(`Setting 'staticComponents' is deprecated. Use 'staticInvokables' instead`);
this.staticComponents = options.staticComponents;
}

if (typeof options.staticHelpers !== 'undefined') {
console.error(`Setting 'staticHelpers' is deprecated. Use 'staticInvokables' instead`);
this.staticHelpers = options.staticHelpers;
}

if (typeof options.staticModifiers !== 'undefined') {
console.error(`Setting 'staticModifiers' is deprecated. Use 'staticInvokables' instead`);
this.staticModifiers = options.staticModifiers;
}
}

@Memoize()
private fastbootJSSrcDir() {
Expand Down Expand Up @@ -269,9 +305,9 @@ export class CompatAppBuilder {
}

let options: CompatResolverOptions['options'] = {
staticHelpers: this.options.staticHelpers,
staticModifiers: this.options.staticModifiers,
staticComponents: this.options.staticComponents,
staticHelpers: this.staticHelpers,
staticModifiers: this.staticModifiers,
staticComponents: this.staticComponents,
allowUnsafeDynamicComponents: this.options.allowUnsafeDynamicComponents,
};

Expand Down Expand Up @@ -1021,12 +1057,7 @@ export class CompatAppBuilder {
transforms.push(macroPlugin as any);
}

if (
this.options.staticComponents ||
this.options.staticHelpers ||
this.options.staticModifiers ||
(globalThis as any).embroider_audit
) {
if (this.staticComponents || this.staticHelpers || this.staticModifiers || (globalThis as any).embroider_audit) {
let opts: ResolverTransformOptions = {
appRoot: resolverConfig.appRoot,
emberVersion: this.emberVersion(),
Expand Down Expand Up @@ -1211,13 +1242,13 @@ export class CompatAppBuilder {
let eagerModules: string[] = [];

let requiredAppFiles = [this.requiredOtherFiles(appFiles)];
if (!this.options.staticComponents) {
if (!this.staticComponents) {
requiredAppFiles.push(appFiles.components);
}
if (!this.options.staticHelpers) {
if (!this.staticHelpers) {
requiredAppFiles.push(appFiles.helpers);
}
if (!this.options.staticModifiers) {
if (!this.staticModifiers) {
requiredAppFiles.push(appFiles.modifiers);
}

Expand Down
3 changes: 2 additions & 1 deletion packages/compat/src/compat-app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import type { Node as BroccoliNode } from 'broccoli-node-api';
import type { Stage, Package } from '@embroider/core';
import { PackageCache, WaitForTrees, RewrittenPackageCache, locateEmbroiderWorkingDir } from '@embroider/core';
import type Options from './options';
import type { CompatOptionsType } from './options';
import { optionsWithDefaults } from './options';
import { Memoize } from 'typescript-memoize';
import { sync as pkgUpSync } from 'pkg-up';
Expand Down Expand Up @@ -41,7 +42,7 @@ interface Group {
export default class CompatApp {
private annotation = '@embroider/compat/app';
private active: CompatAppBuilder | undefined;
readonly options: Required<Options>;
readonly options: CompatOptionsType;

private _publicAssets: { [filePath: string]: string } = Object.create(null);
private _implicitScripts: string[] = [];
Expand Down
11 changes: 7 additions & 4 deletions packages/compat/src/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,12 @@ const defaults = Object.assign(coreWithDefaults(), {
allowUnsafeDynamicComponents: false,
});

export function optionsWithDefaults(options?: Options): Required<Options> {
export type CompatOptionsType = Required<
Omit<Options, 'staticHelpers' | 'staticModifiers' | 'staticComponents' | 'staticInvokables'>
> &
Pick<Options, 'staticHelpers' | 'staticModifiers' | 'staticComponents' | 'staticInvokables'>;

export function optionsWithDefaults(options?: Options): CompatOptionsType {
return Object.assign({}, defaults, options);
}

Expand All @@ -121,10 +126,8 @@ export const recommendedOptions: { [name: string]: Options } = Object.freeze({
optimized: Object.freeze({
staticAddonTrees: true,
staticAddonTestSupportTrees: true,
staticHelpers: true,
staticModifiers: true,
staticComponents: true,
staticEmberSource: true,
allowUnsafeDynamicComponents: false,
staticInvokables: true,
}),
});
4 changes: 1 addition & 3 deletions packages/compat/src/template-tag-codemod.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,7 @@ export default function templateTagCodemod(
compatBuild(emberApp, undefined, {
staticAddonTrees: true,
staticAddonTestSupportTrees: true,
staticComponents: true,
staticHelpers: true,
staticModifiers: true,
staticInvokables: true,
staticEmberSource: true,
amdCompatibility: {
es: [],
Expand Down
6 changes: 3 additions & 3 deletions packages/compat/src/v1-addon.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import rewriteAddonTree from './rewrite-addon-tree';
import { mergeWithAppend } from './merges';
import type { AddonMeta, PackageCache, AddonInstance, AddonTreePath } from '@embroider/core';
import { debug, findTopmostAddon } from '@embroider/core';
import type Options from './options';
import walkSync from 'walk-sync';
import ObserveTree from './observe-tree';
import { isEmbroiderMacrosPlugin } from '@embroider/macros/src/node';
Expand All @@ -34,6 +33,7 @@ import loadAstPlugins from './prepare-htmlbars-ast-plugins';
import getRealAddon from './get-real-addon';
import type { Options as EtcOptions } from 'babel-plugin-ember-template-compilation';
import type CompatApp from './compat-app';
import type { CompatOptionsType } from './options';

const stockTreeNames: AddonTreePath[] = Object.freeze([
'addon',
Expand Down Expand Up @@ -82,7 +82,7 @@ const fastbootPublicationDir = '_fastboot_';
export default class V1Addon {
constructor(
protected addonInstance: AddonInstance,
protected addonOptions: Required<Options>,
protected addonOptions: CompatOptionsType,
protected app: CompatApp,
private packageCache: PackageCache,
private orderIdx: number
Expand Down Expand Up @@ -1094,7 +1094,7 @@ export default class V1Addon {
export interface V1AddonConstructor {
new (
addonInstance: any,
options: Required<Options>,
options: CompatOptionsType,
app: CompatApp,
packageCache: PackageCache,
orderIdx: number
Expand Down
86 changes: 58 additions & 28 deletions packages/core/src/options.ts
Original file line number Diff line number Diff line change
@@ -1,34 +1,62 @@
export default interface Options {
// When true, we statically resolve all template helpers at build time. This
// causes unused helpers to be left out of the build ("tree shaking" of
// helpers).
//
// Defaults to false, which gives you greater compatibility with classic Ember
// apps at the cost of bigger builds.
//
// Enabling this is a prerequisite for route splitting.
/**
* When true, we statically resolve all template helpers at build time. This
* causes unused helpers to be left out of the build ("tree shaking" of
* helpers).
*
* Defaults to false, which gives you greater compatibility with classic Ember
* apps at the cost of bigger builds.
*
* Enabling this is a prerequisite for route splitting.
*
* @deprecated use staticInvokables instead
*/
staticHelpers?: boolean;

// When true, we statically resolve all modifiers at build time. This
// causes unused modifiers to be left out of the build ("tree shaking" of
// modifiers).
//
// Defaults to false, which gives you greater compatibility with classic Ember
// apps at the cost of bigger builds.
//
// Enabling this is a prerequisite for route splitting.
/**
* When true, we statically resolve all modifiers at build time. This
* causes unused modifiers to be left out of the build ("tree shaking" of
* modifiers).
*
* Defaults to false, which gives you greater compatibility with classic Ember
* apps at the cost of bigger builds.
*
* Enabling this is a prerequisite for route splitting.
*
* @deprecated use staticInvokables instead
*/
staticModifiers?: boolean;

// When true, we statically resolve all components at build time. This causes
// unused components to be left out of the build ("tree shaking" of
// components).
//
// Defaults to false, which gives you greater compatibility with classic Ember
// apps at the cost of bigger builds.
//
// Enabling this is a prerequisite for route splitting.
/**
* When true, we statically resolve all components at build time. This causes
* unused components to be left out of the build ("tree shaking" of
* components).
*
* Defaults to false, which gives you greater compatibility with classic Ember
* apps at the cost of bigger builds.
*
* Enabling this is a prerequisite for route splitting.
*
* @deprecated use staticInvokables instead
*/
staticComponents?: boolean;

/**
* When true, we statically resolve all components, modifiers, and helpers (collectively
* knows as Invokables) at build time. This causes any unused Invokables to be left out
* of the build if they are unused i.e. "tree shaking".
*
* Defaults to false which gives you greater compatibility with classic Ember apps at the
* cost of bigger builds.
*
* This setting takes over from `staticHelpers`, `staticModifiers`, and `staticComponents`
* because the Developer Experience was less than ideal if any of these settings did not
* agree i.e. they all needed to be true or they all needed to be false.
*
* Enabling this is a prerequisite for route splitting.
*/
staticInvokables?: boolean;

// Enables per-route code splitting. Any route names that match these patterns
// will be split out of the initial app payload. If you use this, you must
// also add @embroider/router to your app. See [@embroider/router's
Expand Down Expand Up @@ -124,11 +152,13 @@ export default interface Options {
};
}

export function optionsWithDefaults(options?: Options): Required<Options> {
export type CoreOptionsType = Required<
Omit<Options, 'staticHelpers' | 'staticModifiers' | 'staticComponents' | 'staticInvokables'>
> &
Pick<Options, 'staticHelpers' | 'staticModifiers' | 'staticComponents' | 'staticInvokables'>;

export function optionsWithDefaults(options?: Options): CoreOptionsType {
let defaults = {
staticHelpers: false,
staticModifiers: false,
staticComponents: false,
void-mAlex marked this conversation as resolved.
Show resolved Hide resolved
splitAtRoutes: [],
staticAppPaths: [],
skipBabel: [],
Expand Down
4 changes: 1 addition & 3 deletions tests/scenarios/router-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,7 @@ let routerApp = tsAppScenarios.map('router', project => {
return maybeEmbroider(app, {
staticAddonTestSupportTrees: true,
staticAddonTrees: true,
staticComponents: true,
staticHelpers: true,
staticModifiers: true,
staticInvokables: true,
splitAtRoutes: ['split-me'],
skipBabel: [
{
Expand Down
Loading