Skip to content

Use an emit helper (or Object.assign) for JsxSpreadAttributes #7952

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Apr 8, 2016
Merged

Use an emit helper (or Object.assign) for JsxSpreadAttributes #7952

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
bb0ec3b
Use an emit helper for JSX Spread Attributes.
DanielRosenwasser Apr 8, 2016
8ed7c76
Accepted baselines.
DanielRosenwasser Apr 8, 2016
a90d471
Remove unnecessary 'null'/'undefined' check, removed temp for args le…
DanielRosenwasser Apr 8, 2016
67a77dd
Accepted baselines.
DanielRosenwasser Apr 8, 2016
fb6f4ae
Changed emit to use 'Object.prototype.hasOwnProperty'.
DanielRosenwasser Apr 8, 2016
73dbe41
Accepted baselines.
DanielRosenwasser Apr 8, 2016
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: 9 additions & 0 deletions src/compiler/binder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ namespace ts {
let hasAsyncFunctions: boolean;
let hasDecorators: boolean;
let hasParameterDecorators: boolean;
let hasJsxSpreadAttribute: boolean;

// If this file is an external module, then it is automatically in strict-mode according to
// ES6. If it is not an external module, then we'll determine if it is in strict mode or
Expand Down Expand Up @@ -161,6 +162,7 @@ namespace ts {
hasAsyncFunctions = false;
hasDecorators = false;
hasParameterDecorators = false;
hasJsxSpreadAttribute = false;
}

return bindSourceFile;
Expand Down Expand Up @@ -498,6 +500,9 @@ namespace ts {
if (hasAsyncFunctions) {
flags |= NodeFlags.HasAsyncFunctions;
}
if (hasJsxSpreadAttribute) {
flags |= NodeFlags.HasJsxSpreadAttribute;
}
}

node.flags = flags;
Expand Down Expand Up @@ -1298,6 +1303,10 @@ namespace ts {
case SyntaxKind.EnumMember:
return bindPropertyOrMethodOrAccessor(<Declaration>node, SymbolFlags.EnumMember, SymbolFlags.EnumMemberExcludes);

case SyntaxKind.JsxSpreadAttribute:
hasJsxSpreadAttribute = true;
return;

case SyntaxKind.CallSignature:
case SyntaxKind.ConstructSignature:
case SyntaxKind.IndexSignature:
Expand Down
24 changes: 20 additions & 4 deletions src/compiler/emitter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,16 @@ var __extends = (this && this.__extends) || function (d, b) {
d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __());
};`;

const assignHelper = `
var __assign = (this && this.__assign) || Object.assign || function(t) {
Copy link

Choose a reason for hiding this comment

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

Polyfill I found (and what React used) convert t into an object, like:

if (t == null) {
throw new TypeError('Object.assign cannot be called with null or undefined');
}
var result = Object(t);

I'm unsure if this is necessary with current browsers. But https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Object/assign#Polyfill does to.

Copy link
Member Author

Choose a reason for hiding this comment

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

For our uses this should it should be unnecessary anyway.

for (var s, i = 1, n = arguments.length; i < n; i++) {
s = arguments[i];
for (var p in s) if (Object.prototype.hasOwnProperty.call(s, p))
t[p] = s[p];
}
return t;
};`;

// emit output for the __decorate helper function
const decorateHelper = `
var __decorate = (this && this.__decorate) || function (decorators, target, key, desc) {
Expand Down Expand Up @@ -540,6 +550,7 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, ge
let convertedLoopState: ConvertedLoopState;

let extendsEmitted: boolean;
let assignEmitted: boolean;
let decorateEmitted: boolean;
let paramEmitted: boolean;
let awaiterEmitted: boolean;
Expand Down Expand Up @@ -623,6 +634,7 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, ge
decorateEmitted = false;
paramEmitted = false;
awaiterEmitted = false;
assignEmitted = false;
tempFlags = 0;
tempVariables = undefined;
tempParameters = undefined;
Expand Down Expand Up @@ -1259,11 +1271,10 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, ge
}
else {
// Either emit one big object literal (no spread attribs), or
// a call to React.__spread
// a call to the __assign helper
const attrs = openingNode.attributes;
if (forEach(attrs, attr => attr.kind === SyntaxKind.JsxSpreadAttribute)) {
emitExpressionIdentifier(syntheticReactRef);
write(".__spread(");
write("__assign(");

let haveOpenedObjectLiteral = false;
for (let i = 0; i < attrs.length; i++) {
Expand Down Expand Up @@ -7701,11 +7712,16 @@ const _super = (function (geti, seti) {
if (!compilerOptions.noEmitHelpers) {
// Only Emit __extends function when target ES5.
// For target ES6 and above, we can emit classDeclaration as is.
if ((languageVersion < ScriptTarget.ES6) && (!extendsEmitted && node.flags & NodeFlags.HasClassExtends)) {
if (languageVersion < ScriptTarget.ES6 && !extendsEmitted && node.flags & NodeFlags.HasClassExtends) {
writeLines(extendsHelper);
extendsEmitted = true;
}

if (compilerOptions.jsx !== JsxEmit.Preserve && !assignEmitted && (node.flags & NodeFlags.HasJsxSpreadAttribute)) {
writeLines(assignHelper);
assignEmitted = true;
}

if (!decorateEmitted && node.flags & NodeFlags.HasDecorators) {
writeLines(decorateHelper);
if (compilerOptions.emitDecoratorMetadata) {
Expand Down
1 change: 1 addition & 0 deletions src/compiler/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,7 @@ namespace ts {
JavaScriptFile = 1 << 27, // If node was parsed in a JavaScript
ThisNodeOrAnySubNodesHasError = 1 << 28, // If this node or any of its children had an error
HasAggregatedChildData = 1 << 29, // If we've computed data from children and cached it in this node
HasJsxSpreadAttribute = 1 << 30,
Copy link
Member

Choose a reason for hiding this comment

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

Can you log an issue to address the fact that we're now at the effective flag cap for this enum and should break it apart somehow?


Modifier = Export | Ambient | Public | Private | Protected | Static | Abstract | Default | Async,
AccessibilityModifier = Public | Private | Protected,
Expand Down
12 changes: 10 additions & 2 deletions tests/baselines/reference/reactNamespaceJSXEmit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,16 @@ declare var x: any;


//// [reactNamespaceJSXEmit.js]
var __assign = (this && this.__assign) || Object.assign || function(t) {
for (var s, i = 1, n = arguments.length; i < n; i++) {
s = arguments[i];
for (var p in s) if (Object.prototype.hasOwnProperty.call(s, p))
t[p] = s[p];
}
return t;
};
myReactLib.createElement("foo", {data: true});
myReactLib.createElement(Bar, {x: x});
myReactLib.createElement("x-component", null);
myReactLib.createElement(Bar, myReactLib.__spread({}, x));
myReactLib.createElement(Bar, myReactLib.__spread({}, x, {y: 2}));
myReactLib.createElement(Bar, __assign({}, x));
myReactLib.createElement(Bar, __assign({}, x, {y: 2}));
10 changes: 9 additions & 1 deletion tests/baselines/reference/tsxExternalModuleEmit2.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,16 @@ declare var Foo, React;

//// [app.js]
"use strict";
var __assign = (this && this.__assign) || Object.assign || function(t) {
for (var s, i = 1, n = arguments.length; i < n; i++) {
s = arguments[i];
for (var p in s) if (Object.prototype.hasOwnProperty.call(s, p))
t[p] = s[p];
}
return t;
};
var mod_1 = require('mod');
// Should see mod_1['default'] in emit here
React.createElement(Foo, {handler: mod_1["default"]});
// Should see mod_1['default'] in emit here
React.createElement(Foo, React.__spread({}, mod_1["default"]));
React.createElement(Foo, __assign({}, mod_1["default"]));
18 changes: 13 additions & 5 deletions tests/baselines/reference/tsxReactEmit2.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,17 @@ var spreads5 = <div x={p2} {...p1} y={p3}>{p2}</div>;


//// [file.js]
var __assign = (this && this.__assign) || Object.assign || function(t) {
for (var s, i = 1, n = arguments.length; i < n; i++) {
s = arguments[i];
for (var p in s) if (Object.prototype.hasOwnProperty.call(s, p))
t[p] = s[p];
}
return t;
};
var p1, p2, p3;
var spreads1 = React.createElement("div", React.__spread({}, p1), p2);
var spreads2 = React.createElement("div", React.__spread({}, p1), p2);
var spreads3 = React.createElement("div", React.__spread({x: p3}, p1), p2);
var spreads4 = React.createElement("div", React.__spread({}, p1, {x: p3}), p2);
var spreads5 = React.createElement("div", React.__spread({x: p2}, p1, {y: p3}), p2);
var spreads1 = React.createElement("div", __assign({}, p1), p2);
var spreads2 = React.createElement("div", __assign({}, p1), p2);
var spreads3 = React.createElement("div", __assign({x: p3}, p1), p2);
var spreads4 = React.createElement("div", __assign({}, p1, {x: p3}), p2);
var spreads5 = React.createElement("div", __assign({x: p2}, p1, {y: p3}), p2);
10 changes: 9 additions & 1 deletion tests/baselines/reference/tsxReactEmit4.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,15 @@ var openClosed1 = <div>
var spread1 = <div {...p} x={0} />;

//// [file.js]
var __assign = (this && this.__assign) || Object.assign || function(t) {
for (var s, i = 1, n = arguments.length; i < n; i++) {
s = arguments[i];
for (var p in s) if (Object.prototype.hasOwnProperty.call(s, p))
t[p] = s[p];
}
return t;
};
var p;
var openClosed1 = React.createElement("div", null, blah);
// Should emit React.__spread({}, p, {x: 0})
var spread1 = React.createElement("div", React.__spread({}, p, {x: 0}));
var spread1 = React.createElement("div", __assign({}, p, {x: 0}));
10 changes: 9 additions & 1 deletion tests/baselines/reference/tsxReactEmit5.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,16 @@ var spread1 = <div x='' {...foo} y='' />;
//// [file.js]
//// [react-consumer.js]
"use strict";
var __assign = (this && this.__assign) || Object.assign || function(t) {
for (var s, i = 1, n = arguments.length; i < n; i++) {
s = arguments[i];
for (var p in s) if (Object.prototype.hasOwnProperty.call(s, p))
t[p] = s[p];
}
return t;
};
var test_1 = require("./test");
// Should emit test_1.React.createElement
// and React.__spread
var foo;
var spread1 = test_1.React.createElement("div", test_1.React.__spread({x: ''}, foo, {y: ''}));
var spread1 = test_1.React.createElement("div", __assign({x: ''}, foo, {y: ''}));
10 changes: 9 additions & 1 deletion tests/baselines/reference/tsxReactEmit6.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,14 @@ namespace M {

//// [file.js]
//// [react-consumer.js]
var __assign = (this && this.__assign) || Object.assign || function(t) {
for (var s, i = 1, n = arguments.length; i < n; i++) {
s = arguments[i];
for (var p in s) if (Object.prototype.hasOwnProperty.call(s, p))
t[p] = s[p];
}
return t;
};
var M;
(function (M) {
})(M || (M = {}));
Expand All @@ -36,7 +44,7 @@ var M;
// Should emit M.React.createElement
// and M.React.__spread
var foo;
var spread1 = M.React.createElement("div", M.React.__spread({x: ''}, foo, {y: ''}));
var spread1 = M.React.createElement("div", __assign({x: ''}, foo, {y: ''}));
// Quotes
var x = M.React.createElement("div", null, "This \"quote\" thing");
})(M || (M = {}));