Skip to content

Commit

Permalink
[FEAT] Adds isProduction flag for the template compiler
Browse files Browse the repository at this point in the history
This PR adds an `isProduction` flag option to the template compiler that
can be used to change the behavior of the compiler for production
builds. We generally do this already for the JS side of Ember, but not
for templates, and this closes that gap.

Notes:

- Does not expose the `isProduction` flag to AST transforms directly.
  This is purely for removing/replacing internal transforms.
- It works by going over the list of plugins generated by
  `compileOptions` and removing or replacing them. This strategy was
  used for two reasons:
  1. Default plugins are technically public and could be passed in by
     the user. There is already deduping logic to handle this.
  2. Order matters. We have a couple of plugins that need to be
     replaced, and they need to be replaced in the same positition that
     they were added in.
  • Loading branch information
Chris Garrett committed Aug 5, 2020
1 parent 13ff365 commit b05058e
Show file tree
Hide file tree
Showing 5 changed files with 157 additions and 97 deletions.
25 changes: 20 additions & 5 deletions packages/ember-template-compiler/lib/plugins/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,14 @@ import AssertSplattributeExpressions from './assert-splattribute-expression';
import DeprecateSendAction from './deprecate-send-action';
import TransformActionSyntax from './transform-action-syntax';
import TransformAttrsIntoArgs from './transform-attrs-into-args';
import TransformComponentInvocation from './transform-component-invocation';
import {
DebugTransformComponentInvocation,
TransformComponentInvocation,
} from './transform-component-invocation';
import TransformEachInIntoEach from './transform-each-in-into-each';
import TransformEachTrackArray from './transform-each-track-array';
import TransformHasBlockSyntax from './transform-has-block-syntax';
import TransformInElement from './transform-in-element';
import { DebugTransformInElement, TransformInElement } from './transform-in-element';
import TransformLinkTo from './transform-link-to';
import TransformOldClassBindingSyntax from './transform-old-class-binding-syntax';
import TransformQuotedBindingsIntoJustBindings from './transform-quoted-bindings-into-just-bindings';
Expand All @@ -25,7 +28,7 @@ export type APluginFunc = (env: ASTPluginEnvironment) => ASTPlugin | undefined;

// order of plugins is important
const transforms: Array<APluginFunc> = [
TransformComponentInvocation,
DebugTransformComponentInvocation,
TransformOldClassBindingSyntax,
TransformQuotedBindingsIntoJustBindings,
AssertReservedNamedArguments,
Expand All @@ -36,12 +39,12 @@ const transforms: Array<APluginFunc> = [
AssertLocalVariableShadowingHelperInvocation,
TransformLinkTo,
AssertInputHelperWithoutBlock,
TransformInElement,
DebugTransformInElement,
AssertIfHelperWithoutArguments,
AssertSplattributeExpressions,
TransformEachTrackArray,
TransformWrapMountAndOutlet,
];
].filter(v => v !== null);

if (SEND_ACTION) {
transforms.push(DeprecateSendAction);
Expand All @@ -51,4 +54,16 @@ if (!EMBER_NAMED_BLOCKS) {
transforms.push(AssertAgainstNamedBlocks);
}

export const DEBUG_PLUGINS: Array<APluginFunc[] | APluginFunc> = [
[DebugTransformComponentInvocation, TransformComponentInvocation],
[DebugTransformInElement, TransformInElement],
AssertReservedNamedArguments,
AssertLocalVariableShadowingHelperInvocation,
AssertInputHelperWithoutBlock,
AssertIfHelperWithoutArguments,
AssertSplattributeExpressions,
DeprecateSendAction,
AssertAgainstNamedBlocks,
];

export default Object.freeze(transforms);
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { StaticTemplateMeta } from '@ember/-internals/views';
import { AST, ASTPlugin, ASTPluginEnvironment } from '@glimmer/syntax';
import { AST, ASTPluginEnvironment } from '@glimmer/syntax';
import calculateLocationDisplay from '../system/calculate-location-display';
import { Builders } from '../types';
import { isPath, trackLocals } from './utils';
Expand Down Expand Up @@ -122,51 +122,56 @@ import { isPath, trackLocals } from './utils';
@private
@class TransFormComponentInvocation
*/
export default function transformComponentInvocation(env: ASTPluginEnvironment): ASTPlugin {
let { moduleName } = env.meta as StaticTemplateMeta;
let { builders: b } = env.syntax;
function buildTransformComponentInvocation(isProduction: boolean) {
return (env: ASTPluginEnvironment) => {
let { moduleName } = env.meta as StaticTemplateMeta;
let { builders: b } = env.syntax;

let { hasLocal, node } = trackLocals();
let { hasLocal, node } = trackLocals();

let isAttrs = false;
let isAttrs = false;

return {
name: 'transform-component-invocation',
return {
name: 'transform-component-invocation',

visitor: {
Program: node,
visitor: {
Program: node,

ElementNode: {
keys: {
attributes: {
enter() {
isAttrs = true;
},
ElementNode: {
keys: {
attributes: {
enter() {
isAttrs = true;
},

exit() {
isAttrs = false;
exit() {
isAttrs = false;
},
},
},

children: node,
children: node,
},
},
},

BlockStatement(node: AST.BlockStatement) {
if (isBlockInvocation(node, hasLocal)) {
wrapInComponent(moduleName, node, b);
}
},
BlockStatement(node: AST.BlockStatement) {
if (isBlockInvocation(node, hasLocal)) {
wrapInComponent(moduleName, node, b, isProduction);
}
},

MustacheStatement(node: AST.MustacheStatement): AST.Node | void {
if (!isAttrs && isInlineInvocation(node, hasLocal)) {
wrapInComponent(moduleName, node, b);
}
MustacheStatement(node: AST.MustacheStatement): AST.Node | void {
if (!isAttrs && isInlineInvocation(node, hasLocal)) {
wrapInComponent(moduleName, node, b, isProduction);
}
},
},
},
};
};
}

export const DebugTransformComponentInvocation = buildTransformComponentInvocation(false);
export const TransformComponentInvocation = buildTransformComponentInvocation(true);

function isInlineInvocation(
node: AST.MustacheStatement,
hasLocal: (k: string) => boolean
Expand Down Expand Up @@ -223,9 +228,12 @@ function wrapInAssertion(moduleName: string, node: AST.PathExpression, b: Builde
function wrapInComponent(
moduleName: string,
node: AST.MustacheStatement | AST.BlockStatement,
b: Builders
b: Builders,
isProduction: boolean
) {
let component = wrapInAssertion(moduleName, node.path as AST.PathExpression, b);
let component = isProduction
? node.path
: wrapInAssertion(moduleName, node.path as AST.PathExpression, b);
node.path = b.path('component');
node.params.unshift(component);
}
121 changes: 63 additions & 58 deletions packages/ember-template-compiler/lib/plugins/transform-in-element.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,86 +42,91 @@ import { isPath } from './utils';
@private
@class TransformInElement
*/
export default function transformInElement(env: ASTPluginEnvironment): ASTPlugin {
let { moduleName } = env.meta as StaticTemplateMeta;
let { builders: b } = env.syntax;
function buildTransformInElement(isProduction: boolean) {
return (env: ASTPluginEnvironment): ASTPlugin => {
let { moduleName } = env.meta as StaticTemplateMeta;
let { builders: b } = env.syntax;

return {
name: 'transform-in-element',
return {
name: 'transform-in-element',

visitor: {
BlockStatement(node: AST.BlockStatement) {
if (!isPath(node.path)) return;
visitor: {
BlockStatement(node: AST.BlockStatement) {
if (!isPath(node.path)) return;

if (node.path.original === 'in-element') {
if (EMBER_GLIMMER_IN_ELEMENT) {
let originalValue = node.params[0];
if (node.path.original === 'in-element') {
if (EMBER_GLIMMER_IN_ELEMENT) {
let originalValue = node.params[0];

if (originalValue) {
let subExpr = b.sexpr('-in-el-null', [originalValue]);
if (originalValue && !isProduction) {
let subExpr = b.sexpr('-in-el-null', [originalValue]);

node.params.shift();
node.params.unshift(subExpr);
node.params.shift();
node.params.unshift(subExpr);
}

node.hash.pairs.forEach(pair => {
if (pair.key === 'insertBefore') {
assert(
`Can only pass null to insertBefore in in-element, received: ${JSON.stringify(
pair.value
)}`,
pair.value.type === 'NullLiteral' || pair.value.type === 'UndefinedLiteral'
);
}
});
} else {
assert(assertMessage(moduleName, node));
}
} else if (node.path.original === '-in-element') {
if (EMBER_GLIMMER_IN_ELEMENT) {
let sourceInformation = calculateLocationDisplay(moduleName, node.loc);
deprecate(
`The use of the private \`{{-in-element}}\` is deprecated, please refactor to the public \`{{in-element}}\`. ${sourceInformation}`,
false,
{
id: 'glimmer.private-in-element',
until: '3.25.0',
}
);
}

node.hash.pairs.forEach(pair => {
node.path.original = 'in-element';
node.path.parts = ['in-element'];

// replicate special hash arguments added here:
// https://github.com/glimmerjs/glimmer-vm/blob/ba9b37d44b85fa1385eeeea71910ff5798198c8e/packages/%40glimmer/syntax/lib/parser/handlebars-node-visitors.ts#L340-L363
let needsInsertBefore = true;
let hash = node.hash;
hash.pairs.forEach(pair => {
if (pair.key === 'insertBefore') {
assert(
`Can only pass null to insertBefore in in-element, received: ${JSON.stringify(
`Can only pass a null or undefined literals to insertBefore in -in-element, received: ${JSON.stringify(
pair.value
)}`,
pair.value.type === 'NullLiteral' || pair.value.type === 'UndefinedLiteral'
);

needsInsertBefore = false;
}
});
} else {
assert(assertMessage(moduleName, node));
}
} else if (node.path.original === '-in-element') {
if (EMBER_GLIMMER_IN_ELEMENT) {
let sourceInformation = calculateLocationDisplay(moduleName, node.loc);
deprecate(
`The use of the private \`{{-in-element}}\` is deprecated, please refactor to the public \`{{in-element}}\`. ${sourceInformation}`,
false,
{
id: 'glimmer.private-in-element',
until: '3.25.0',
}
);
}

node.path.original = 'in-element';
node.path.parts = ['in-element'];

// replicate special hash arguments added here:
// https://github.com/glimmerjs/glimmer-vm/blob/ba9b37d44b85fa1385eeeea71910ff5798198c8e/packages/%40glimmer/syntax/lib/parser/handlebars-node-visitors.ts#L340-L363
let needsInsertBefore = true;
let hash = node.hash;
hash.pairs.forEach(pair => {
if (pair.key === 'insertBefore') {
assert(
`Can only pass a null or undefined literals to insertBefore in -in-element, received: ${JSON.stringify(
pair.value
)}`,
pair.value.type === 'NullLiteral' || pair.value.type === 'UndefinedLiteral'
);

needsInsertBefore = false;
// Maintain compatibility with previous -in-element behavior (defaults to append, not clear)
if (needsInsertBefore) {
let nullLiteral = b.literal('NullLiteral', null);
let nextSibling = b.pair('insertBefore', nullLiteral);
hash.pairs.push(nextSibling);
}
});

// Maintain compatibility with previous -in-element behavior (defaults to append, not clear)
if (needsInsertBefore) {
let nullLiteral = b.literal('NullLiteral', null);
let nextSibling = b.pair('insertBefore', nullLiteral);
hash.pairs.push(nextSibling);
}
}
},
},
},
};
};
}

export const DebugTransformInElement = buildTransformInElement(false);
export const TransformInElement = buildTransformInElement(true);

function assertMessage(moduleName: string, node: AST.BlockStatement) {
let sourceInformation = calculateLocationDisplay(moduleName, node.loc);

Expand Down
20 changes: 19 additions & 1 deletion packages/ember-template-compiler/lib/system/compile-options.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { assign } from '@ember/polyfills';
import { PrecompileOptions } from '@glimmer/compiler';
import { AST, ASTPlugin, ASTPluginEnvironment, Syntax } from '@glimmer/syntax';
import PLUGINS, { APluginFunc } from '../plugins/index';
import PLUGINS, { APluginFunc, DEBUG_PLUGINS } from '../plugins/index';
import COMPONENT_NAME_SIMPLE_DASHERIZE_CACHE from './dasherize-component-name';

type PluginFunc = APluginFunc & {
Expand All @@ -17,6 +17,7 @@ export interface CompileOptions {
meta?: any;
moduleName?: string | undefined;
plugins?: Plugins | undefined;
isProduction?: boolean;
}

export default function compileOptions(_options: Partial<CompileOptions> = {}): PrecompileOptions {
Expand All @@ -40,9 +41,26 @@ export default function compileOptions(_options: Partial<CompileOptions> = {}):
let pluginsToAdd = potententialPugins.filter(plugin => {
return options.plugins!.ast.indexOf(plugin) === -1;
});

options.plugins.ast = providedPlugins.concat(pluginsToAdd);
}

if (options.isProduction) {
let plugins = options.plugins.ast;

for (let plugin of DEBUG_PLUGINS) {
if (Array.isArray(plugin)) {
let [debugPlugin, prodPlugin] = plugin;

let index = plugins.indexOf(debugPlugin);
plugins.splice(index, 1, prodPlugin);
} else {
let index = plugins.indexOf(plugin);
plugins.splice(index, 1);
}
}
}

return options;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
registerPlugin,
unregisterPlugin,
} from '../../index';
import { DEBUG_PLUGINS } from '../../lib/plugins/index';
import { moduleFor, AbstractTestCase, RenderingTestCase } from 'internal-test-helpers';

moduleFor(
Expand All @@ -24,6 +25,19 @@ moduleFor(
assert.ok(plugins.indexOf(plugin) > -1, `includes ${plugin}`);
}
}

['@test isProduction removes and replaces debug plugins'](assert) {
let plugins = compileOptions({ isProduction: true }).plugins.ast;

for (let plugin of DEBUG_PLUGINS) {
if (Array.isArray(plugin)) {
assert.equal(plugins.indexOf(plugin[0]), -1, 'debug plugin removed');
assert.notEqual(plugins.indexOf(plugin[1]), -1, 'prod plugin added');
} else {
assert.equal(plugins.indexOf(plugin), -1, 'debug plugin removed');
}
}
}
}
);

Expand Down

0 comments on commit b05058e

Please sign in to comment.