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(ngcc): handle new __spreadArrays tslib helper #33617

Closed
wants to merge 1 commit into from
Closed

fix(ngcc): handle new __spreadArrays tslib helper #33617

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
11 changes: 7 additions & 4 deletions packages/compiler-cli/ngcc/src/host/esm5_host.ts
View file
Open in desktop
Expand Up @@ -627,10 +627,13 @@ function getTsHelperFn(node: ts.NamedDeclaration): TsHelperFn|null {
stripDollarSuffix(node.name.text) :
null;

if (name === '__spread') {
return TsHelperFn.Spread;
} else {
return null;
switch (name) {
case '__spread':
return TsHelperFn.Spread;
case '__spreadArrays':
return TsHelperFn.SpreadArrays;
default:
return null;
}
}

Expand Down
74 changes: 73 additions & 1 deletion packages/compiler-cli/ngcc/test/host/esm5_host_spec.ts
View file
Open in desktop
Expand Up @@ -1651,7 +1651,7 @@ runInEachFileSystem(() => {
it('should recognize TypeScript __spread helper function declaration', () => {
const file: TestFile = {
name: _('/declaration.d.ts'),
contents: `export declare function __spread(...args: any[]): any[];`,
contents: `export declare function __spread(...args: any[][]): any[];`,
};
loadTestFiles([file]);
const {program} = makeTestBundleProgram(file.name);
Expand Down Expand Up @@ -1711,6 +1711,78 @@ runInEachFileSystem(() => {
expect(definition.helper).toBe(TsHelperFn.Spread);
expect(definition.parameters.length).toEqual(0);
});

it('should recognize TypeScript __spreadArrays helper function declaration', () => {
const file: TestFile = {
name: _('/declaration.d.ts'),
contents: `export declare function __spreadArrays(...args: any[][]): any[];`,
};
loadTestFiles([file]);
const {program} = makeTestBundleProgram(file.name);
const host = new Esm5ReflectionHost(new MockLogger(), false, program.getTypeChecker());

const node =
getDeclaration(program, file.name, '__spreadArrays', isNamedFunctionDeclaration) !;

const definition = host.getDefinitionOfFunction(node) !;
expect(definition.node).toBe(node);
expect(definition.body).toBeNull();
expect(definition.helper).toBe(TsHelperFn.SpreadArrays);
expect(definition.parameters.length).toEqual(0);
});

it('should recognize TypeScript __spreadArrays helper function implementation', () => {
const file: TestFile = {
name: _('/implementation.js'),
contents: `
var __spreadArrays = (this && this.__spreadArrays) || function () {
for (var s = 0, i = 0, il = arguments.length; i < il; i++) s += arguments[i].length;
for (var r = Array(s), k = 0, i = 0; i < il; i++)
for (var a = arguments[i], j = 0, jl = a.length; j < jl; j++, k++)
r[k] = a[j];
return r;
};`,
};
loadTestFiles([file]);
const {program} = makeTestBundleProgram(file.name);
const host = new Esm5ReflectionHost(new MockLogger(), false, program.getTypeChecker());

const node =
getDeclaration(program, file.name, '__spreadArrays', ts.isVariableDeclaration) !;

const definition = host.getDefinitionOfFunction(node) !;
expect(definition.node).toBe(node);
expect(definition.body).toBeNull();
expect(definition.helper).toBe(TsHelperFn.SpreadArrays);
expect(definition.parameters.length).toEqual(0);
});

it('should recognize TypeScript __spreadArrays helper function implementation when suffixed',
() => {
const file: TestFile = {
name: _('/implementation.js'),
contents: `
var __spreadArrays$2 = (this && this.__spreadArrays$2) || function () {
for (var s = 0, i = 0, il = arguments.length; i < il; i++) s += arguments[i].length;
for (var r = Array(s), k = 0, i = 0; i < il; i++)
for (var a = arguments[i], j = 0, jl = a.length; j < jl; j++, k++)
r[k] = a[j];
return r;
};`,
};
loadTestFiles([file]);
const {program} = makeTestBundleProgram(file.name);
const host = new Esm5ReflectionHost(new MockLogger(), false, program.getTypeChecker());

const node =
getDeclaration(program, file.name, '__spreadArrays$2', ts.isVariableDeclaration) !;

const definition = host.getDefinitionOfFunction(node) !;
expect(definition.node).toBe(node);
expect(definition.body).toBeNull();
expect(definition.helper).toBe(TsHelperFn.SpreadArrays);
expect(definition.parameters.length).toEqual(0);
});
});

describe('getImportOfIdentifier()', () => {
Expand Down
10 changes: 6 additions & 4 deletions packages/compiler-cli/src/ngtsc/partial_evaluator/src/ts_helpers.ts
View file
Open in desktop
Expand Up @@ -15,10 +15,12 @@ import {ResolvedValue, ResolvedValueArray} from './result';

export function evaluateTsHelperInline(
helper: TsHelperFn, node: ts.Node, args: ResolvedValueArray): ResolvedValue {
if (helper === TsHelperFn.Spread) {
return evaluateTsSpreadHelper(node, args);
} else {
throw new Error(`Cannot evaluate unknown helper ${helper} inline`);
switch (helper) {
case TsHelperFn.Spread:
case TsHelperFn.SpreadArrays:
return evaluateTsSpreadHelper(node, args);
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 consider evaluating SpreadArrays differently, given that this was introduced because the two scenarios emit different downlevelled code?

Copy link
Member

Choose a reason for hiding this comment

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

function spreadArrays() {
  for (var s = 0, i = 0, il = arguments.length; i < il; i++) s += arguments[i].length;
  for (var r = Array(s), k = 0, i = 0; i < il; i++)
    for (var a = arguments[i], j = 0, jl = a.length; j < jl; j++, k++)
      r[k] = a[j];
  return r;
}
function spread() {
  for (var ar = [], i = 0; i < arguments.length; i++) ar = ar.concat(__read(arguments[i]));
  return ar;
}

Our current helper does this:

function evaluateTsSpreadHelper(node: ts.Node, args: ResolvedValueArray): ResolvedValueArray {
  const result: ResolvedValueArray = [];
  for (const arg of args) {
    if (arg instanceof DynamicValue) {
      result.push(DynamicValue.fromDynamicInput(node, arg));
    } else if (Array.isArray(arg)) {
      result.push(...arg);
    } else {
      result.push(arg);
    }
  }
  return result;
}

Copy link
Member

Choose a reason for hiding this comment

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

I guess that if the arg is already an array then it has been dealt with elsewhere and any downlevelling of the iterator is irrelevant.

default:
throw new Error(`Cannot evaluate unknown helper ${helper} inline`);
}
}

Expand Down
34 changes: 29 additions & 5 deletions packages/compiler-cli/src/ngtsc/partial_evaluator/test/evaluator_spec.ts
View file
Open in desktop
Expand Up @@ -514,7 +514,28 @@ runInEachFileSystem(() => {
{
name: _('/node_modules/tslib/index.d.ts'),
contents: `
export declare function __spread(...args: any[]): any[];
export declare function __spread(...args: any[][]): any[];
`
},
]);
const reflectionHost = new TsLibAwareReflectionHost(checker);
const evaluator = new PartialEvaluator(reflectionHost, checker);
const value = evaluator.evaluate(expression);
expect(value).toEqual([1, 2, 3]);
});

it('should evaluate TypeScript __spreadArrays helper', () => {
const {checker, expression} = makeExpression(
`
import * as tslib from 'tslib';
const a = [1];
const b = [2, 3];
`,
'tslib.__spreadArrays(a, b)', [
{
name: _('/node_modules/tslib/index.d.ts'),
contents: `
export declare function __spreadArrays(...args: any[][]): any[];
`
},
]);
Expand Down Expand Up @@ -612,10 +633,13 @@ runInEachFileSystem(() => {
function getTsHelperFn(node: ts.FunctionDeclaration): TsHelperFn|null {
const name = node.name !== undefined && ts.isIdentifier(node.name) && node.name.text;

if (name === '__spread') {
return TsHelperFn.Spread;
} else {
return null;
switch (name) {
case '__spread':
return TsHelperFn.Spread;
case '__spreadArrays':
return TsHelperFn.SpreadArrays;
default:
return null;
}
}
});
4 changes: 4 additions & 0 deletions packages/compiler-cli/src/ngtsc/reflection/src/host.ts
View file
Open in desktop
Expand Up @@ -336,6 +336,10 @@ export enum TsHelperFn {
* Indicates the `__spread` function.
*/
Spread,
/**
* Indicates the `__spreadArrays` function.
*/
SpreadArrays,
}

/**
Expand Down