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

Correctly cache tagged template objects in modules #18300

Merged
merged 29 commits into from
Oct 3, 2017
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
c4b1a9d
Update emit for tagged templates to use a per-site cached template ob…
DanielRosenwasser Sep 7, 2017
166af8c
Accepted baselines.
DanielRosenwasser Sep 7, 2017
aa634ba
Added printer test for 'new (f().x)'.
DanielRosenwasser Sep 14, 2017
4beb9b0
Accepted (incorrect) baselines.
DanielRosenwasser Sep 14, 2017
6a9fa83
Parenthesize new'd expressions based on the leftmost node (or the fir…
DanielRosenwasser Sep 14, 2017
c966059
Accepted baselines.
DanielRosenwasser Sep 14, 2017
8fbb304
Add a test case for conditional expressions just in case.
DanielRosenwasser Sep 14, 2017
7871e08
Accepted baselines.
DanielRosenwasser Sep 14, 2017
9f669d0
Explicit fall-through.
DanielRosenwasser Sep 14, 2017
e9c6dfe
Remove freezing behavior from tagged template helper.
DanielRosenwasser Sep 15, 2017
1656790
Accepted baselines.
DanielRosenwasser Sep 15, 2017
b137f24
%s/getTemplateObject/makeTemplateObject
DanielRosenwasser Sep 18, 2017
5565709
Accepted baselines.
DanielRosenwasser Sep 18, 2017
9907453
Merge branch 'master' into correctlyCacheTaggedTemplates
DanielRosenwasser Sep 21, 2017
1cb5eb9
Merge branch 'master' into correctlyCacheTaggedTemplates
DanielRosenwasser Sep 25, 2017
886a29b
Added tests for import helpers with & without a declared template obj…
DanielRosenwasser Sep 26, 2017
1841afe
Ensure that the import helper is checked for tagged templates, and up…
DanielRosenwasser Sep 28, 2017
0b7538d
Accepted baselines.
DanielRosenwasser Sep 28, 2017
b406d54
git Merge branch 'master' into correctlyCacheTaggedTemplates
DanielRosenwasser Sep 28, 2017
4ec1643
Fall back to old behavior for tagged template emit in global files.
DanielRosenwasser Sep 28, 2017
d039942
Accepted baselines.
DanielRosenwasser Sep 28, 2017
5da45fb
Addressed code review feedback.
DanielRosenwasser Sep 30, 2017
f94bded
Added test for module & global examples.
DanielRosenwasser Sep 30, 2017
81b3e85
Accepted baselines.
DanielRosenwasser Sep 30, 2017
a23d1bf
Updated helper for marginally better minification.
DanielRosenwasser Sep 30, 2017
e2c6aac
Accepted baselines.
DanielRosenwasser Sep 30, 2017
b80b2ee
Move first/last shortcuts closer to the end of the helper flags.
DanielRosenwasser Oct 2, 2017
babe3cb
Flatten the '__makeTemplateObject' helper to use less vertical screen…
DanielRosenwasser Oct 3, 2017
8fd638c
Accepted baselines.
DanielRosenwasser Oct 3, 2017
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
4 changes: 4 additions & 0 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16587,6 +16587,9 @@ namespace ts {
}

function checkTaggedTemplateExpression(node: TaggedTemplateExpression): Type {
if (languageVersion < ScriptTarget.ES2015) {
checkExternalEmitHelpers(node, ExternalEmitHelpers.GetTemplateObject);
}
return getReturnTypeOfSignature(getResolvedSignature(node));
}

Expand Down Expand Up @@ -23919,6 +23922,7 @@ namespace ts {
case ExternalEmitHelpers.AsyncDelegator: return "__asyncDelegator";
case ExternalEmitHelpers.AsyncValues: return "__asyncValues";
case ExternalEmitHelpers.ExportStar: return "__exportStar";
case ExternalEmitHelpers.GetTemplateObject: return "__getTemplateObject";
default: Debug.fail("Unrecognized helper");
}
}
Expand Down
67 changes: 55 additions & 12 deletions src/compiler/transformers/es2015.ts
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,16 @@ namespace ts {
let currentSourceFile: SourceFile;
let currentText: string;
let hierarchyFacts: HierarchyFacts;
let taggedTemplateStringDeclarations: VariableDeclaration[];
function recordTaggedTemplateString(temp: Identifier) {
const decl = createVariableDeclaration(temp);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

taggedTemplateStringDeclarations = append(taggedTemplateStringDeclarations, createVariableDeclaration(temp));

if (!taggedTemplateStringDeclarations) {
taggedTemplateStringDeclarations = [decl];
}
else {
taggedTemplateStringDeclarations.push(decl);
}
}

/**
* Used to track if we are emitting body of the converted loop
Expand Down Expand Up @@ -307,6 +317,7 @@ namespace ts {

currentSourceFile = undefined;
currentText = undefined;
taggedTemplateStringDeclarations = undefined;
hierarchyFacts = HierarchyFacts.None;
return visited;
}
Expand Down Expand Up @@ -520,6 +531,11 @@ namespace ts {
addCaptureThisForNodeIfNeeded(statements, node);
statementOffset = addCustomPrologue(statements, node.statements, statementOffset, visitor);
addRange(statements, visitNodes(node.statements, visitor, isStatement, statementOffset));
if (taggedTemplateStringDeclarations) {
statements.push(
createVariableStatement(/*modifiers*/ undefined,
createVariableDeclarationList(taggedTemplateStringDeclarations)));
}
addRange(statements, endLexicalEnvironment());
exitSubtree(ancestorFacts, HierarchyFacts.None, HierarchyFacts.None);
return updateSourceFileNode(
Expand Down Expand Up @@ -3637,10 +3653,12 @@ namespace ts {
const tag = visitNode(node.tag, visitor, isExpression);

// Allocate storage for the template site object
const temp = createTempVariable(hoistVariableDeclaration);
const temp = createTempVariable(recordTaggedTemplateString);

// Build up the template arguments and the raw and cooked strings for the template.
const templateArguments: Expression[] = [temp];
// We start out with 'undefined' for the first argument and revisit later
// to avoid walking over the template string twice and shifting all our arguments over after the fact.
const templateArguments: Expression[] = [undefined];
const cookedStrings: Expression[] = [];
const rawStrings: Expression[] = [];
const template = node.template;
Expand All @@ -3658,16 +3676,14 @@ namespace ts {
}
}

// NOTE: The parentheses here is entirely optional as we are now able to auto-
// parenthesize when rebuilding the tree. This should be removed in a
// future version. It is here for now to match our existing emit.
return createParen(
inlineExpressions([
createAssignment(temp, createArrayLiteral(cookedStrings)),
createAssignment(createPropertyAccess(temp, "raw"), createArrayLiteral(rawStrings)),
createCall(tag, /*typeArguments*/ undefined, templateArguments)
])
);
// Initialize the template object if necessary
templateArguments[0] = createLogicalOr(
temp,
createAssignment(
temp,
createTemplateObjectHelper(context, createArrayLiteral(cookedStrings), createArrayLiteral(rawStrings))));

return createCall(tag, /*typeArguments*/ undefined, templateArguments);
}

/**
Expand Down Expand Up @@ -4036,6 +4052,18 @@ namespace ts {
);
}

function createTemplateObjectHelper(context: TransformationContext, cooked: ArrayLiteralExpression, raw: ArrayLiteralExpression) {
context.requestEmitHelper(templateObjectHelper);
return createCall(
getHelperName("__getTemplateObject"),
/*typeArguments*/ undefined,
[
cooked,
raw
]
);
}

const extendsHelper: EmitHelper = {
name: "typescript:extends",
scoped: false,
Expand All @@ -4052,4 +4080,19 @@ namespace ts {
};
})();`
};

const templateObjectHelper: EmitHelper = {
name: "typescript:getTemplateObject",
scoped: false,
priority: 0,
text: `
var __getTemplateObject = (this && this.__getTemplateObject) || function (cooked, raw) {
if (Object.freeze && Object.defineProperty) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you need the defineProperty call? I suppose it makes the property non-enumerable...

return Object.freeze(Object.defineProperty(cooked, "raw", { value: Object.freeze(raw) }));
}
cooked.raw = raw;
return cooked;
};`
};

}
33 changes: 17 additions & 16 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4217,22 +4217,23 @@ namespace ts {
*/
/* @internal */
export const enum ExternalEmitHelpers {
Extends = 1 << 0, // __extends (used by the ES2015 class transformation)
Assign = 1 << 1, // __assign (used by Jsx and ESNext object spread transformations)
Rest = 1 << 2, // __rest (used by ESNext object rest transformation)
Decorate = 1 << 3, // __decorate (used by TypeScript decorators transformation)
Metadata = 1 << 4, // __metadata (used by TypeScript decorators transformation)
Param = 1 << 5, // __param (used by TypeScript decorators transformation)
Awaiter = 1 << 6, // __awaiter (used by ES2017 async functions transformation)
Generator = 1 << 7, // __generator (used by ES2015 generator transformation)
Values = 1 << 8, // __values (used by ES2015 for..of and yield* transformations)
Read = 1 << 9, // __read (used by ES2015 iterator destructuring transformation)
Spread = 1 << 10, // __spread (used by ES2015 array spread and argument list spread transformations)
Await = 1 << 11, // __await (used by ES2017 async generator transformation)
AsyncGenerator = 1 << 12, // __asyncGenerator (used by ES2017 async generator transformation)
AsyncDelegator = 1 << 13, // __asyncDelegator (used by ES2017 async generator yield* transformation)
AsyncValues = 1 << 14, // __asyncValues (used by ES2017 for..await..of transformation)
ExportStar = 1 << 15, // __exportStar (used by CommonJS/AMD/UMD module transformation)
Extends = 1 << 0, // __extends (used by the ES2015 class transformation)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this change is an example of why aligned comments make me sad :(

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:(

Assign = 1 << 1, // __assign (used by Jsx and ESNext object spread transformations)
Rest = 1 << 2, // __rest (used by ESNext object rest transformation)
Decorate = 1 << 3, // __decorate (used by TypeScript decorators transformation)
Metadata = 1 << 4, // __metadata (used by TypeScript decorators transformation)
Param = 1 << 5, // __param (used by TypeScript decorators transformation)
Awaiter = 1 << 6, // __awaiter (used by ES2017 async functions transformation)
Generator = 1 << 7, // __generator (used by ES2015 generator transformation)
Values = 1 << 8, // __values (used by ES2015 for..of and yield* transformations)
Read = 1 << 9, // __read (used by ES2015 iterator destructuring transformation)
Spread = 1 << 10, // __spread (used by ES2015 array spread and argument list spread transformations)
Await = 1 << 11, // __await (used by ES2017 async generator transformation)
AsyncGenerator = 1 << 12, // __asyncGenerator (used by ES2017 async generator transformation)
AsyncDelegator = 1 << 13, // __asyncDelegator (used by ES2017 async generator yield* transformation)
AsyncValues = 1 << 14, // __asyncValues (used by ES2017 for..await..of transformation)
ExportStar = 1 << 15, // __exportStar (used by CommonJS/AMD/UMD module transformation)
GetTemplateObject = 1 << 16, // __getTemplateObject (used for constructing template string array objects)

// Helpers included by ES2015 for..of
ForOfIncludes = Values,
Expand Down
11 changes: 9 additions & 2 deletions tests/baselines/reference/asOperator3.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,19 @@ var g = tag `Hello ${123} World` as string;
var h = tag `Hello` as string;

//// [asOperator3.js]
var __getTemplateObject = (this && this.__getTemplateObject) || function (cooked, raw) {
if (Object.freeze && Object.defineProperty) {
return Object.freeze(Object.defineProperty(cooked, "raw", { value: Object.freeze(raw) }));
}
cooked.raw = raw;
return cooked;
};
var a = "" + (123 + 456);
var b = "leading " + (123 + 456);
var c = 123 + 456 + " trailing";
var d = "Hello " + 123 + " World";
var e = "Hello";
var f = 1 + (1 + " end of string");
var g = (_a = ["Hello ", " World"], _a.raw = ["Hello ", " World"], tag(_a, 123));
var h = (_b = ["Hello"], _b.raw = ["Hello"], tag(_b));
var g = tag(_a || (_a = __getTemplateObject(["Hello ", " World"], ["Hello ", " World"])), 123);
var h = tag(_b || (_b = __getTemplateObject(["Hello"], ["Hello"])));
var _a, _b;
9 changes: 8 additions & 1 deletion tests/baselines/reference/asOperatorASI.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,21 @@ as(Foo); // should emit


//// [asOperatorASI.js]
var __getTemplateObject = (this && this.__getTemplateObject) || function (cooked, raw) {
if (Object.freeze && Object.defineProperty) {
return Object.freeze(Object.defineProperty(cooked, "raw", { value: Object.freeze(raw) }));
}
cooked.raw = raw;
return cooked;
};
var Foo = /** @class */ (function () {
function Foo() {
}
return Foo;
}());
// Example 1
var x = 10;
(_a = ["Hello world"], _a.raw = ["Hello world"], as(_a)); // should not error
as(_a || (_a = __getTemplateObject(["Hello world"], ["Hello world"]))); // should not error
// Example 2
var y = 20;
as(Foo); // should emit
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,18 @@ function f(...args: any[]) {
f `\x0D${ "Interrupted CRLF" }\x0A`;

//// [taggedTemplateStringsHexadecimalEscapes.js]
var __getTemplateObject = (this && this.__getTemplateObject) || function (cooked, raw) {
if (Object.freeze && Object.defineProperty) {
return Object.freeze(Object.defineProperty(cooked, "raw", { value: Object.freeze(raw) }));
}
cooked.raw = raw;
return cooked;
};
function f() {
var args = [];
for (var _i = 0; _i < arguments.length; _i++) {
args[_i] = arguments[_i];
}
}
(_a = ["\r", "\n"], _a.raw = ["\\x0D", "\\x0A"], f(_a, "Interrupted CRLF"));
f(_a || (_a = __getTemplateObject(["\r", "\n"], ["\\x0D", "\\x0A"])), "Interrupted CRLF");
var _a;
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,18 @@ function f(...x: any[]) {
f `0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 2028 2029 0085 t v f b r n`

//// [taggedTemplateStringsPlainCharactersThatArePartsOfEscapes01.js]
var __getTemplateObject = (this && this.__getTemplateObject) || function (cooked, raw) {
if (Object.freeze && Object.defineProperty) {
return Object.freeze(Object.defineProperty(cooked, "raw", { value: Object.freeze(raw) }));
}
cooked.raw = raw;
return cooked;
};
function f() {
var x = [];
for (var _i = 0; _i < arguments.length; _i++) {
x[_i] = arguments[_i];
}
}
(_a = ["0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 2028 2029 0085 t v f b r n"], _a.raw = ["0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 2028 2029 0085 t v f b r n"], f(_a));
f(_a || (_a = __getTemplateObject(["0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 2028 2029 0085 t v f b r n"], ["0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 2028 2029 0085 t v f b r n"])));
var _a;
Loading