Ts 3.7-beta support by filipesilva · Pull Request #32962 · angular/angular

filipesilva · 2019-10-02T14:21:29Z

Just testing to see what breaks if #32946 is followed up by a 3.7-beta update.

PR Checklist

Please check if your PR fulfills the following requirements:

The commit message follows our guidelines: https://github.com/angular/angular/blob/master/CONTRIBUTING.md#commit
Tests for the changes have been added (for bug fixes / features)
Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

Bugfix
Feature
Code style update (formatting, local variables)
Refactoring (no functional changes, no api changes)
Build related changes
CI related changes
Documentation content changes
angular.io application / infrastructure changes
Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

Yes
No

Other information

googlebot · 2019-10-02T14:21:33Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

mary-poppins · 2019-10-02T14:30:57Z

You can preview ee6157b at https://pr32962-ee6157b.ngbuilds.io/.

filipesilva · 2019-10-02T14:39:09Z

microsoft/TypeScript#33693 is a breaking change that affects us. It's easy to work around though:

before:

          const typeReference = type as ts.TypeReference;
          if (typeReference.typeArguments && typeReference.typeArguments.length === 1) {
            return typeReference.typeArguments[0].symbol;
          }

after

          const typeReference = type as ts.TypeReference;
          const typeArguments = this.checker.getTypeArguments(typeReference);
          if (typeArguments.length === 1) {
            return typeArguments[0].symbol;
          }

lppedd · 2019-10-02T21:49:17Z

Hi @filipesilva !
I was trying out TS 3.7 on my Angular application. If it can help in some way, I'm receiving an error only on dev builds, while having importHelpers: true and thus using tslib. This doesn't happen with --prod or when importHelpers: false.

ReferenceError: __importDefault is not defined

At, for exampe, template: __importDefault

ErrorComponent = Object(tslib__WEBPACK_IMPORTED_MODULE_0__["__decorate"])([
    Object(_angular_core__WEBPACK_IMPORTED_MODULE_1__["Component"])({
        selector: 'tea-error',
        template: __importDefault(__webpack_require__(/*! raw-loader!./error.component.html */ "./node_modules/raw-loader/dist/cjs.js!./src/main/webapp/app/error/error.component.html")).default,
        changeDetection: _angular_core__WEBPACK_IMPORTED_MODULE_1__["ChangeDetectionStrategy"].OnPush
    })
], ErrorComponent);

Obviously I know I should not use 3.7, it's just "for fun".

mary-poppins · 2019-10-07T10:31:12Z

You can preview 5838059 at https://pr32962-5838059.ngbuilds.io/.

filipesilva · 2019-10-07T10:37:35Z

Heya @lppedd, thanks for letting us know. I think the integration tests in this repo should catch the same error.

mary-poppins · 2019-10-07T10:43:11Z

You can preview 4ac0c72 at https://pr32962-4ac0c72.ngbuilds.io/.

mary-poppins · 2019-10-07T14:46:35Z

You can preview a0c0e20 at https://pr32962-a0c0e20.ngbuilds.io/.

filipesilva · 2019-10-07T14:50:54Z

See #32962 (comment)

filipesilva · 2019-10-07T15:34:20Z

Fix for

packages/core/src/di/injectable.ts(12,9): error TS2440: Import declaration conflicts with local declaration of 'InjectableType'.

Listed under breaking changes in https://devblogs.microsoft.com/typescript/announcing-typescript-3-7-beta/.

filipesilva · 2019-10-07T15:34:32Z

Fix for

packages/core/src/render3/instructions/shared.ts(1272,9): error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?

Listed under breaking changes in https://devblogs.microsoft.com/typescript/announcing-typescript-3-7-beta/.

The if guard is there to differentiate between DirectiveDef and ComponentDef, by testing to see if def has a template property. This is probably still relevant, so a better fix might be

Suggested change

exportsMap[''] = index;

if ((def as ComponentDef<any>).template !== undefined) exportsMap[''] = index;

or

Suggested change

exportsMap[''] = index;

if (!!(def as ComponentDef<any>).template) exportsMap[''] = index;

(which is the alternative workaround in the notes)

The cast to ComponentDef<any> looks odd to me since ComponentDef always has a template. It's means "say that def is a ComponentDef, is the template property defined?" to which it makes sense that TS would complain.

Shouldn't the cast be something different then?

Yeah, that pattern is a bit misleading. In this case, the cast is there as an assumption of it indeed being a ComponentDef, then checking if template actually exists to confirm.

This pattern is often used in functions with type predicates:

export function isComponentDef<T>(def: DirectiveDef<T>|ComponentDef<T>): def is ComponentDef<T> { return (def as ComponentDef<T>).template !== undefined; }

In this case however, the compiler requires the return type to be boolean, so the original guard as written before you changed it would not have compiled in the type guard. I guess the extra function is omitted for code size (although I'm not sure on that one).

The benefit of the "assumed cast" is that there's proper reference tracking, i.e. IDEs know that the template property read does in fact correspond with ComponentDef.template, so e.g. renaming template will correctly update the isComponentDef function.

filipesilva · 2019-10-07T15:36:09Z

Compilation currently fails with this error:

Error: rest parameter does not resolve to a reference type
    at ModuleTypeTranslator.getFunctionTypeJSDoc (/home/circleci/.cache/bazel/_bazel_circleci/9ce5c2144ecf75d11717c0aa41e45a8d/sandbox/processwrapper-sandbox/378/execroot/angular/bazel-out/host/bin/packages/bazel/src/ngc-wrapped/ngc-wrapped.sh.runfiles/npm/node_modules/tsickle/src/module_type_translator.js:424:39)
    at visitFunctionLikeDeclaration (/home/circleci/.cache/bazel/_bazel_circleci/9ce5c2144ecf75d11717c0aa41e45a8d/sandbox/processwrapper-sandbox/378/execroot/angular/bazel-out/host/bin/packages/bazel/src/ngc-wrapped/ngc-wrapped.sh.runfiles/npm/node_modules/tsickle/src/jsdoc_transformer.js:527:75)
    at visitor (/home/circleci/.cache/bazel/_bazel_circleci/9ce5c2144ecf75d11717c0aa41e45a8d/sandbox/processwrapper-sandbox/378/execroot/angular/bazel-out/host/bin/packages/bazel/src/ngc-wrapped/ngc-wrapped.sh.runfiles/npm/node_modules/tsickle/src/jsdoc_transformer.js:913:36)
    at visitNodes (/home/circleci/.cache/bazel/_bazel_circleci/9ce5c2144ecf75d11717c0aa41e45a8d/sandbox/processwrapper-sandbox/378/execroot/angular/bazel-out/host/bin/packages/bazel/src/ngc-wrapped/ngc-wrapped.sh.runfiles/npm/node_modules/typescript/lib/typescript.js:70546:48)
    at visitLexicalEnvironment (/home/circleci/.cache/bazel/_bazel_circleci/9ce5c2144ecf75d11717c0aa41e45a8d/sandbox/processwrapper-sandbox/378/execroot/angular/bazel-out/host/bin/packages/bazel/src/ngc-wrapped/ngc-wrapped.sh.runfiles/npm/node_modules/typescript/lib/typescript.js:70579:22)
...

This corresponds to the following code in tsickle:

                        if (type.flags & ts.TypeFlags.Object &&
                            type.objectFlags & ts.ObjectFlags.Reference) {
                            const typeRef = type;
                            if (!typeRef.typeArguments) {
                                throw new Error('rest parameter does not resolve to a reference type');
                            }
                            type = typeRef.typeArguments[0];
                        }

This seems to be another instance of #32962 (comment).

cc @mprobst @IgorMinar

Reported as angular/tsickle#1096

lppedd · 2019-10-11T09:29:09Z

After updating to CLI 8.3.9 I started receiving the following error on build (both dev and prod).
Reverting to 8.3.8 fixes it.

[error] Error: Debug Failure. Invalid cast. The supplied value [object Object] did not pass the test 'isBindableStaticAccessExpression'.
    at Object.cast (/home/edoardo/IdeaProjects/tria-smartea/node_modules/typescript/lib/typescript.js:1488:25)
    at bindSpecialPropertyAssignment (/home/edoardo/IdeaProjects/tria-smartea/node_modules/typescript/lib/typescript.js:31557:53)
    at bindWorker (/home/edoardo/IdeaProjects/tria-smartea/node_modules/typescript/lib/typescript.js:31131:29)
    at bind (/home/edoardo/IdeaProjects/tria-smartea/node_modules/typescript/lib/typescript.js:31010:13)
    at visitNode (/home/edoardo/IdeaProjects/tria-smartea/node_modules/typescript/lib/typescript.js:17672:24)
    at Object.forEachChild (/home/edoardo/IdeaProjects/tria-smartea/node_modules/typescript/lib/typescript.js:17894:21)
    at bindEachChild (/home/edoardo/IdeaProjects/tria-smartea/node_modules/typescript/lib/typescript.js:29627:16)
    at bindBinaryExpressionFlow (/home/edoardo/IdeaProjects/tria-smartea/node_modules/typescript/lib/typescript.js:30330:17)
    at bindChildrenWorker (/home/edoardo/IdeaProjects/tria-smartea/node_modules/typescript/lib/typescript.js:29688:21)
    at bindChildren (/home/edoardo/IdeaProjects/tria-smartea/node_modules/typescript/lib/typescript.js:29597:17)
    at bind (/home/edoardo/IdeaProjects/tria-smartea/node_modules/typescript/lib/typescript.js:31021:21)
    at visitNode (/home/edoardo/IdeaProjects/tria-smartea/node_modules/typescript/lib/typescript.js:17672:24)
    at Object.forEachChild (/home/edoardo/IdeaProjects/tria-smartea/node_modules/typescript/lib/typescript.js:17892:24)
    at bindEachChild (/home/edoardo/IdeaProjects/tria-smartea/node_modules/typescript/lib/typescript.js:29627:16)
    at bindBinaryExpressionFlow (/home/edoardo/IdeaProjects/tria-smartea/node_modules/typescript/lib/typescript.js:30330:17)
    at bindChildrenWorker (/home/edoardo/IdeaProjects/tria-smartea/node_modules/typescript/lib/typescript.js:29688:21)

Latest 3.7-dev.

filipesilva · 2019-10-15T17:07:06Z

@lppedd the __importDefault problem should be fixed in angular/angular-cli#15847.

I'm not sure what that last error is. Was that on ng build?

lppedd · 2019-10-15T17:31:01Z

@filipesilva thanks! And yes, that last Invalid cast error pops up on ng build.
If you need other info let me know, I'll update to 8.3.9 again.

googlebot · 2019-10-22T10:11:38Z

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

mary-poppins · 2019-10-22T10:17:47Z

You can preview 42aef61 at https://pr32962-42aef61.ngbuilds.io/.

gkalpak · 2020-01-14T17:12:12Z

This seems to have been superceded by #33717. Can we close this, @filipesilva?

angular-automatic-lock-bot · 2020-02-14T16:08:58Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

googlebot added the cla: no label Oct 2, 2019

atscott added the area: build & ci Related the build and CI infrastructure of the project label Oct 2, 2019

ngbot Bot added this to the needsTriage milestone Oct 2, 2019

filipesilva commented Oct 7, 2019

View reviewed changes

Comment thread packages/compiler-cli/src/diagnostics/typescript_symbols.ts Outdated

Copy link
Copy Markdown

Contributor Author

filipesilva Oct 7, 2019

Choose a reason for hiding this comment

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

See #32962 (comment)

filipesilva commented Oct 7, 2019

View reviewed changes

filipesilva mentioned this pull request Oct 7, 2019

Support TypeScript 3.7 angular/tsickle#1096

Closed

feat: typescript 3.7-beta support

42aef61

googlebot added cla: yes and removed cla: no labels Oct 22, 2019

gkalpak mentioned this pull request Nov 5, 2019

Support TypeScript 3.7 #33596

Closed

alan-agius4 mentioned this pull request Nov 5, 2019

Support TypeScript 3.7 angular/angular-cli#16071

Closed

andrius-pra mentioned this pull request Nov 10, 2019

build: typescript 3.7 support #33717

Closed

14 tasks

andrius-pra mentioned this pull request Jan 5, 2020

refactor(language-service): adds Symbol#typeArguments and does cleanup #34571

Closed

7 tasks

filipesilva closed this Jan 14, 2020

angular-automatic-lock-bot Bot locked and limited conversation to collaborators Feb 14, 2020

	exportsMap[''] = index;
	if ((def as ComponentDef<any>).template !== undefined) exportsMap[''] = index;

	exportsMap[''] = index;
	if (!!(def as ComponentDef<any>).template) exportsMap[''] = index;

Conversation

filipesilva commented Oct 2, 2019

PR Checklist

PR Type

What is the current behavior?

What is the new behavior?

Does this PR introduce a breaking change?

Other information

Uh oh!

googlebot commented Oct 2, 2019

Uh oh!

mary-poppins commented Oct 2, 2019

Uh oh!

filipesilva commented Oct 2, 2019

Uh oh!

lppedd commented Oct 2, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mary-poppins commented Oct 7, 2019

Uh oh!

filipesilva commented Oct 7, 2019

Uh oh!

mary-poppins commented Oct 7, 2019

Uh oh!

mary-poppins commented Oct 7, 2019

Uh oh!

filipesilva Oct 7, 2019

Choose a reason for hiding this comment

Uh oh!

filipesilva Oct 7, 2019

Choose a reason for hiding this comment

Uh oh!

filipesilva Oct 7, 2019

Choose a reason for hiding this comment

Uh oh!

JoostK Oct 7, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

filipesilva Oct 7, 2019

Choose a reason for hiding this comment

Uh oh!

JoostK Oct 7, 2019

Choose a reason for hiding this comment

Uh oh!

filipesilva commented Oct 7, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lppedd commented Oct 11, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

filipesilva commented Oct 15, 2019

Uh oh!

lppedd commented Oct 15, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

googlebot commented Oct 22, 2019

Uh oh!

mary-poppins commented Oct 22, 2019

Uh oh!

gkalpak commented Jan 14, 2020

Uh oh!

angular-automatic-lock-bot Bot commented Feb 14, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

lppedd commented Oct 2, 2019 •

edited

Loading

JoostK Oct 7, 2019 •

edited

Loading

filipesilva commented Oct 7, 2019 •

edited

Loading

lppedd commented Oct 11, 2019 •

edited

Loading

lppedd commented Oct 15, 2019 •

edited

Loading