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.

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

Jump to bottom

fix(linker): minified umd fixes #41747

Closed
wants to merge 3 commits into from
Closed

fix(linker): minified umd fixes #41747

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ea85ff5
fix(compiler-cli): allow linker to process minified booleans
petebacondarwin Apr 21, 2021
a7c015b
fix(compiler-cli): match string indexed partial declarations
petebacondarwin Apr 21, 2021
3edd153
fixup! fix(compiler-cli): allow linker to process minified booleans
petebacondarwin Apr 22, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
23 changes: 20 additions & 3 deletions packages/compiler-cli/linker/babel/src/ast/babel_ast_host.ts
View file
Open in desktop
Expand Up @@ -38,11 +38,18 @@ export class BabelAstHost implements AstHost<t.Expression> {
return num.value;
}

isBooleanLiteral = t.isBooleanLiteral;
isBooleanLiteral(bool: t.Expression): boolean {
return t.isBooleanLiteral(bool) || isMinifiedBooleanLiteral(bool);
}

parseBooleanLiteral(bool: t.Expression): boolean {
assert(bool, t.isBooleanLiteral, 'a boolean literal');
return bool.value;
if (t.isBooleanLiteral(bool)) {
return bool.value;
} else if (isMinifiedBooleanLiteral(bool)) {
return !bool.argument.value;
} else {
throw new FatalLinkerError(bool, 'Unsupported syntax, expected a boolean literal.');
}
}

isArrayLiteral = t.isArrayExpression;
Expand Down Expand Up @@ -165,3 +172,13 @@ type ArgumentType = t.CallExpression['arguments'][number];
function isNotSpreadArgument(arg: ArgumentType): arg is Exclude<ArgumentType, t.SpreadElement> {
return !t.isSpreadElement(arg);
}

type MinifiedBooleanLiteral = t.Expression&t.UnaryExpression&{argument: t.NumericLiteral};

/**
* Return true if the node is either `!0` or `!1`.
*/
function isMinifiedBooleanLiteral(node: t.Expression): node is MinifiedBooleanLiteral {
return t.isUnaryExpression(node) && node.prefix && node.operator === '!' &&
t.isNumericLiteral(node.argument) && (node.argument.value === 0 || node.argument.value === 1);
}
2 changes: 2 additions & 0 deletions packages/compiler-cli/linker/babel/src/es2015_linker_plugin.ts
View file
Open in desktop
Expand Up @@ -139,6 +139,8 @@ function getCalleeName(call: NodePath<t.CallExpression>): string|null {
return callee.name;
} else if (t.isMemberExpression(callee) && t.isIdentifier(callee.property)) {
return callee.property.name;
} else if (t.isMemberExpression(callee) && t.isStringLiteral(callee.property)) {
Copy link
Member

Choose a reason for hiding this comment

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

Should we also support this case in AstHost.getSymbolName?

Copy link
Member Author

Choose a reason for hiding this comment

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

This case was not reported as problematic in the original issue, so I was being conservative in adding support. I would rather wait to see if we get any further issues. It is an easy fix to add in the future if it comes up.

return callee.property.value;
} else {
return null;
}
Expand Down
12 changes: 12 additions & 0 deletions packages/compiler-cli/linker/babel/test/ast/babel_ast_host_spec.ts
View file
Open in desktop
Expand Up @@ -96,6 +96,11 @@ describe('BabelAstHost', () => {
expect(host.isBooleanLiteral(expr('false'))).toBe(true);
});

it('should return true if the expression is a minified boolean literal', () => {
expect(host.isBooleanLiteral(expr('!0'))).toBe(true);
expect(host.isBooleanLiteral(expr('!1'))).toBe(true);
petebacondarwin marked this conversation as resolved.
Show resolved Hide resolved
});

it('should return false if the expression is not a boolean literal', () => {
expect(host.isBooleanLiteral(expr('"moo"'))).toBe(false);
expect(host.isBooleanLiteral(expr('\'moo\''))).toBe(false);
Expand All @@ -106,6 +111,8 @@ describe('BabelAstHost', () => {
expect(host.isBooleanLiteral(expr('null'))).toBe(false);
expect(host.isBooleanLiteral(expr('\'a\' + \'b\''))).toBe(false);
expect(host.isBooleanLiteral(expr('\`moo\`'))).toBe(false);
expect(host.isBooleanLiteral(expr('!2'))).toBe(false);
expect(host.isBooleanLiteral(expr('~1'))).toBe(false);
});
});

Expand All @@ -115,6 +122,11 @@ describe('BabelAstHost', () => {
expect(host.parseBooleanLiteral(expr('false'))).toEqual(false);
});

it('should extract a minified boolean value', () => {
expect(host.parseBooleanLiteral(expr('!0'))).toEqual(true);
expect(host.parseBooleanLiteral(expr('!1'))).toEqual(false);
});

it('should error if the value is not a boolean literal', () => {
expect(() => host.parseBooleanLiteral(expr('"moo"')))
.toThrowError('Unsupported syntax, expected a boolean literal.');
Expand Down
9 changes: 7 additions & 2 deletions packages/compiler-cli/linker/babel/test/es2015_linker_plugin_spec.ts
View file
Open in desktop
Expand Up @@ -70,9 +70,10 @@ describe('createEs2015LinkerPlugin()', () => {
[
'var core;',
`ɵɵngDeclareDirective({minVersion: '0.0.0-PLACEHOLDER', version: '0.0.0-PLACEHOLDER', ngImport: core, x: 1});`,
`ɵɵngDeclareComponent({minVersion: '0.0.0-PLACEHOLDER', version: '0.0.0-PLACEHOLDER', ngImport: core, foo: () => ɵɵngDeclareDirective({minVersion: '0.0.0-PLACEHOLDER', version: '0.0.0-PLACEHOLDER', ngImport: core, x: 2})});`,
`i0.ɵɵngDeclareComponent({minVersion: '0.0.0-PLACEHOLDER', version: '0.0.0-PLACEHOLDER', ngImport: core, foo: () => ɵɵngDeclareDirective({minVersion: '0.0.0-PLACEHOLDER', version: '0.0.0-PLACEHOLDER', ngImport: core, x: 2})});`,
`x.qux(() => ɵɵngDeclareDirective({minVersion: '0.0.0-PLACEHOLDER', version: '0.0.0-PLACEHOLDER', ngImport: core, x: 3}));`,
'spread(...x);',
`i0['ɵɵngDeclareDirective']({minVersion: '0.0.0-PLACEHOLDER', version: '0.0.0-PLACEHOLDER', ngImport: core, x: 4});`,
].join('\n'),
{
plugins: [createEs2015LinkerPlugin({fileSystem, logger})],
Expand All @@ -93,7 +94,11 @@ describe('createEs2015LinkerPlugin()', () => {
[
'ɵɵngDeclareDirective',
'{minVersion:\'0.0.0-PLACEHOLDER\',version:\'0.0.0-PLACEHOLDER\',ngImport:core,x:3}'
]
],
[
'ɵɵngDeclareDirective',
'{minVersion:\'0.0.0-PLACEHOLDER\',version:\'0.0.0-PLACEHOLDER\',ngImport:core,x:4}'
],
]);
});

Expand Down
8 changes: 7 additions & 1 deletion packages/compiler-cli/linker/src/ast/ast_host.ts
View file
Open in desktop
Expand Up @@ -36,11 +36,17 @@ export interface AstHost<TExpression> {
parseNumericLiteral(num: TExpression): number;

/**
* Return `true` if the given expression is a boolean literal, or false otherwise.
* Return `true` if the given expression can be considered a boolean literal, or false otherwise.
*
* Note that this should also cover the special case of some minified code where `true` and
* `false` are replaced by `!0` and `!1` respectively.
*/
isBooleanLiteral(node: TExpression): boolean;
/**
* Parse the boolean value from the given expression, or throw if it is not a boolean literal.
*
* Note that this should also cover the special case of some minified code where `true` and
* `false` are replaced by `!0` and `!1` respectively.
*/
parseBooleanLiteral(bool: TExpression): boolean;

Expand Down
30 changes: 26 additions & 4 deletions packages/compiler-cli/linker/src/ast/typescript/typescript_ast_host.ts
View file
Open in desktop
Expand Up @@ -47,13 +47,18 @@ export class TypeScriptAstHost implements AstHost<ts.Expression> {
return parseInt(num.text);
}

isBooleanLiteral(node: ts.Expression): node is ts.FalseLiteral|ts.TrueLiteral {
return node.kind === ts.SyntaxKind.TrueKeyword || node.kind === ts.SyntaxKind.FalseKeyword;
isBooleanLiteral(node: ts.Expression): boolean {
return isBooleanLiteral(node) || isMinifiedBooleanLiteral(node);
}

parseBooleanLiteral(bool: ts.Expression): boolean {
assert(bool, this.isBooleanLiteral, 'a boolean literal');
return bool.kind === ts.SyntaxKind.TrueKeyword;
if (isBooleanLiteral(bool)) {
return bool.kind === ts.SyntaxKind.TrueKeyword;
} else if (isMinifiedBooleanLiteral(bool)) {
return !(+bool.operand.text);
} else {
throw new FatalLinkerError(bool, 'Unsupported syntax, expected a boolean literal.');
}
}

isArrayLiteral = ts.isArrayLiteralExpression;
Expand Down Expand Up @@ -160,3 +165,20 @@ function isNotSpreadElement(e: ts.Expression|ts.SpreadElement): e is ts.Expressi
function isPropertyName(e: ts.PropertyName): e is ts.Identifier|ts.StringLiteral|ts.NumericLiteral {
return ts.isIdentifier(e) || ts.isStringLiteral(e) || ts.isNumericLiteral(e);
}

/**
* Return true if the node is either `true` or `false` literals.
*/
function isBooleanLiteral(node: ts.Expression): node is ts.TrueLiteral|ts.FalseLiteral {
return node.kind === ts.SyntaxKind.TrueKeyword || node.kind === ts.SyntaxKind.FalseKeyword;
}

type MinifiedBooleanLiteral = ts.PrefixUnaryExpression&{operand: ts.NumericLiteral};

/**
* Return true if the node is either `!0` or `!1`.
*/
function isMinifiedBooleanLiteral(node: ts.Expression): node is MinifiedBooleanLiteral {
return ts.isPrefixUnaryExpression(node) && node.operator === ts.SyntaxKind.ExclamationToken &&
ts.isNumericLiteral(node.operand) && (node.operand.text === '0' || node.operand.text === '1');
}
12 changes: 12 additions & 0 deletions packages/compiler-cli/linker/test/ast/typescript/typescript_ast_host_spec.ts
View file
Open in desktop
Expand Up @@ -94,6 +94,11 @@ describe('TypeScriptAstHost', () => {
expect(host.isBooleanLiteral(expr('false'))).toBe(true);
});

it('should return true if the expression is a minified boolean literal', () => {
expect(host.isBooleanLiteral(expr('!0'))).toBe(true);
expect(host.isBooleanLiteral(expr('!1'))).toBe(true);
});

it('should return false if the expression is not a boolean literal', () => {
expect(host.isBooleanLiteral(expr('"moo"'))).toBe(false);
expect(host.isBooleanLiteral(expr('\'moo\''))).toBe(false);
Expand All @@ -104,6 +109,8 @@ describe('TypeScriptAstHost', () => {
expect(host.isBooleanLiteral(expr('null'))).toBe(false);
expect(host.isBooleanLiteral(expr('\'a\' + \'b\''))).toBe(false);
expect(host.isBooleanLiteral(expr('\`moo\`'))).toBe(false);
expect(host.isBooleanLiteral(expr('!2'))).toBe(false);
expect(host.isBooleanLiteral(expr('~1'))).toBe(false);
});
});

Expand All @@ -113,6 +120,11 @@ describe('TypeScriptAstHost', () => {
expect(host.parseBooleanLiteral(expr('false'))).toEqual(false);
});

it('should extract a minified boolean value', () => {
expect(host.parseBooleanLiteral(expr('!0'))).toEqual(true);
expect(host.parseBooleanLiteral(expr('!1'))).toEqual(false);
});

it('should error if the value is not a boolean literal', () => {
expect(() => host.parseBooleanLiteral(expr('"moo"')))
.toThrowError('Unsupported syntax, expected a boolean literal.');
Expand Down