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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ngcc doesn't produce valid es5 code #32665

Closed
dawidgarus opened this issue Sep 13, 2019 · 6 comments

Comments

@dawidgarus
Copy link

@dawidgarus dawidgarus commented Sep 13, 2019

馃悶 bug report

Affected Package

The issue is caused by package @angular/....

@angular/forms, @angular/common

Is this a regression?

Yes, it works without ivy

Description

A clear and concise description of the problem...

Code produced by ngcc is not valid es5.
Examples:
Fragment of node_modules@angular\forms_ivy_ngcc_\fesm5\forms.js

const _c0 = ["ng-untouched", "ng-touched", "ng-pristine", "ng-dirty", "ng-valid", "ng-invalid", "ng-pending"];
const _c1 = ["novalidate", ""];

const!
Fragment of node_modules@angular\common_ivy_ngcc_\fesm5\common.js

NgForOf.ngDirectiveDef = 傻ngcc0.傻傻defineDirective({ type: NgForOf, selectors: [["", "ngFor", "", "ngForOf", ""]], factory: function NgForOf_Factory(t) { return new (t || NgForOf)(傻ngcc0.傻傻directiveInject(ViewContainerRef), 傻ngcc0.傻傻directiveInject(TemplateRef), 傻ngcc0.傻傻directiveInject(IterableDiffers)); }, inputs: { ngForOf: "ngForOf", ngForTrackBy: "ngForTrackBy", ngForTemplate: "ngForTemplate" } });
/*@__PURE__*/ 傻ngcc0.傻setClassMetadata(NgForOf, [{
        type: Directive,
        args: [{ selector: '[ngFor][ngForOf]' }]
    }], function () { return [{ type: ViewContainerRef }, { type: TemplateRef }, { type: IterableDiffers }]; }, { _viewContainer: [], _template: [], _differs: [], _ngForOfDirty: [], _differ: [], ngForOf: [{
            type: Input
        }], ngForTrackBy: [{
            type: Input
        }], ngForTrackBy: [], ngForTemplate: [{
            type: Input
        }], ngDoCheck: [], _applyChanges: [], _perViewChange: [] });
    return NgForOf;
}());

Notice double ngForTrackBy. PhantomJS thinks it's Syntax Error.

馃敩 Minimal Reproduction

Compile any project which used ngFor or NG_VALUE_ACCESSOR

馃實 Your Environment

Angular Version:

{
  "dependencies": {
    "@angular/animations": "8.1.1",
    "@angular/common": "8.1.1",
    "@angular/compiler": "8.1.1",
    "@angular/core": "8.1.1",
    "@angular/forms": "8.1.1",
    "@angular/platform-browser": "8.1.1",
    "@angular/platform-browser-dynamic": "8.1.1",
    "@angular/router": "8.1.1",
    "core-js": "2.4.1",
    "rxjs": "6.4.0",
    "rxjs-compat": "6.4.0",
    "tslib": "1.9.1",
    "zone.js": "0.9.1"
  },
  "devDependencies": {
    "@angular/cli": "8.1.1",
    "@angular/compiler-cli": "8.1.1",
    "@angular/language-service": "8.1.1",
    "@ngtools/webpack": "^8.1.1",
    "codelyzer": "^5.0.0",
    "gulp": "^4.0.2",
    "typescript": "3.4.5",
    "webpack": "^4.35.2"
  }
}

Anything else relevant?

@kara kara added the comp: ngcc label Sep 13, 2019
@ngbot ngbot bot added this to the needsTriage milestone Sep 13, 2019
@alxhub

This comment has been minimized.

Copy link
Contributor

@alxhub alxhub commented Oct 10, 2019

It's legal ES5 to repeat a property name inside an object literal, but this is a weird issue. We'll investigate :)

@trotyl

This comment has been minimized.

Copy link
Contributor

@trotyl trotyl commented Oct 11, 2019

It's legal ES5 to repeat a property name inside an object literal

It's only valid in ES5 sloppy mode, but not in strict mode, and valid in both sloppy and strict mode after ES6. Given all module files are forced to be strict, that's not a valid ES5 code.

@dawidgarus

This comment has been minimized.

Copy link
Author

@dawidgarus dawidgarus commented Oct 11, 2019

What's also weird is that property is repeated but with different value. I don't know what should be the correct value, but it's worth to investigate whether the correct value is actually used. Note that minifiers like terser will remove duplicates.

@gkalpak

This comment has been minimized.

Copy link
Member

@gkalpak gkalpak commented Oct 22, 2019

The duplicate value is due to ngcc not handling getters/setters correctly. This is tracked in #30569.
We should still look into the const thing.

@gkalpak

This comment has been minimized.

Copy link
Member

@gkalpak gkalpak commented Oct 22, 2019

Interestingly, according to MDN (and empirical data 馃榿), const is supported in IE11.
(It's still not valid ES5 and we should look into it, of course.)

@dawidgarus

This comment has been minimized.

Copy link
Author

@dawidgarus dawidgarus commented Oct 23, 2019

@gkalpak yes, ES5 code generated by ngcc works in IE 11, that's probably why nobody notice anything weird before. However, I had notice that PhantomJS, and perhaps other tools that except ES5 code, is not as forgiving and it breaks my CI/CD pipeline.

@gkalpak gkalpak self-assigned this Oct 30, 2019
gkalpak added a commit to gkalpak/angular that referenced this issue Oct 30, 2019
Previously, ngcc's `Renderer` would emit some constants in the processed
files which were "reified" by a generic `ts.Printer`, that could print
invalid ES5 code (e.g. `const` declarations). As a result, the generated
code would break when run on browsers that do not support the emitted
format.

This commit fixes it by adding a `printNode()` method to
`RenderingFormatter`, which can print nodes in a suitable format for the
corresponding `RenderingFormatter`.

Fixes angular#32665
gkalpak added a commit to gkalpak/angular that referenced this issue Oct 30, 2019
Previously, ngcc's `Renderer` would emit some constants in the processed
files which were "reified" by a generic `ts.Printer`, that could print
invalid ES5 code (e.g. `const` declarations). As a result, the generated
code would break when run on browsers that do not support the emitted
format.

This commit fixes it by adding a `printNode()` method to
`RenderingFormatter`, which can print nodes in a suitable format for the
corresponding `RenderingFormatter`.

Fixes angular#32665
gkalpak added a commit to gkalpak/angular that referenced this issue Oct 31, 2019
Previously, ngcc's `Renderer` would emit some constants in the processed
files which were "reified" by a generic `ts.Printer`, that could print
invalid ES5 code (e.g. `const` declarations). As a result, the generated
code would break when run on browsers that do not support the emitted
format.

This commit fixes it by adding a `printNode()` method to
`RenderingFormatter`, which can print nodes in a suitable format for the
corresponding `RenderingFormatter`.

Fixes angular#32665
gkalpak added a commit to gkalpak/angular that referenced this issue Oct 31, 2019
Previously, ngcc's `Renderer` would emit some constants in the processed
files which were "reified" by a generic `ts.Printer`, that could print
invalid ES5 code (e.g. `const` declarations). As a result, the generated
code would break when run on browsers that do not support the emitted
format.

This commit fixes it by adding a `printNode()` method to
`RenderingFormatter`, which can print nodes in a suitable format for the
corresponding `RenderingFormatter`.

Fixes angular#32665
@JoostK JoostK added the state: has PR label Nov 2, 2019
gkalpak added a commit to gkalpak/angular that referenced this issue Nov 4, 2019
Previously, ngcc's `Renderer` would emit some constants in the processed
files which were "reified" by a generic `ts.Printer`, that could print
invalid ES5 code (e.g. `const` declarations). As a result, the generated
code would break when run on browsers that do not support the emitted
format.

This commit fixes it by adding a `printNode()` method to
`RenderingFormatter`, which can print nodes in a suitable format for the
corresponding `RenderingFormatter`.

Fixes angular#32665
gkalpak added a commit to gkalpak/angular that referenced this issue Nov 4, 2019
Previously, ngcc's `Renderer` would add some constants in the processed
files which were emitted as ES2015 code (e.g. `const` declarations).
This would result in invalid ES5 generated code that would break when
run on browsers that do not support the emitted format.

This commit fixes it by passing an extra hint to `translateExpression()`
and `translateStatement()` helper methods to translate the code to ES5
format if necessary.

Fixes angular#32665
gkalpak added a commit to gkalpak/angular that referenced this issue Nov 4, 2019
Previously, ngcc's `Renderer` would add some constants in the processed
files which were emitted as ES2015 code (e.g. `const` declarations).
This would result in invalid ES5 generated code that would break when
run on browsers that do not support the emitted format.

This commit fixes it by passing an extra hint to `translateExpression()`
and `translateStatement()` helper methods to translate the code to ES5
format if necessary.

Fixes angular#32665
gkalpak added a commit to gkalpak/angular that referenced this issue Nov 4, 2019
Previously, ngcc's `Renderer` would add some constants in the processed
files which were emitted as ES2015 code (e.g. `const` declarations).
This would result in invalid ES5 generated code that would break when
run on browsers that do not support the emitted format.

This commit fixes it by adding a `printStatement()` method to
`RenderingFormatter`, which can convert statements to JavaScript code in
a suitable format for the corresponding `RenderingFormatter`.
Additionally, the `translateExpression()` and `translateStatement()`
ngtsc helper methods are augmented to accept an extra hint to know
whether the code needs to be translated to ES5 format or not.

Fixes angular#32665
gkalpak added a commit to gkalpak/angular that referenced this issue Nov 4, 2019
Previously, ngcc's `Renderer` would add some constants in the processed
files which were emitted as ES2015 code (e.g. `const` declarations).
This would result in invalid ES5 generated code that would break when
run on browsers that do not support the emitted format.

This commit fixes it by adding a `printStatement()` method to
`RenderingFormatter`, which can convert statements to JavaScript code in
a suitable format for the corresponding `RenderingFormatter`.
Additionally, the `translateExpression()` and `translateStatement()`
ngtsc helper methods are augmented to accept an extra hint to know
whether the code needs to be translated to ES5 format or not.

Fixes angular#32665
gkalpak added a commit to gkalpak/angular that referenced this issue Nov 4, 2019
Previously, ngcc's `Renderer` would add some constants in the processed
files which were emitted as ES2015 code (e.g. `const` declarations).
This would result in invalid ES5 generated code that would break when
run on browsers that do not support the emitted format.

This commit fixes it by adding a `printStatement()` method to
`RenderingFormatter`, which can convert statements to JavaScript code in
a suitable format for the corresponding `RenderingFormatter`.
Additionally, the `translateExpression()` and `translateStatement()`
ngtsc helper methods are augmented to accept an extra hint to know
whether the code needs to be translated to ES5 format or not.

Fixes angular#32665
gkalpak added a commit to gkalpak/angular that referenced this issue Nov 4, 2019
Previously, ngcc's `Renderer` would add some constants in the processed
files which were emitted as ES2015 code (e.g. `const` declarations).
This would result in invalid ES5 generated code that would break when
run on browsers that do not support the emitted format.

This commit fixes it by adding a `printStatement()` method to
`RenderingFormatter`, which can convert statements to JavaScript code in
a suitable format for the corresponding `RenderingFormatter`.
Additionally, the `translateExpression()` and `translateStatement()`
ngtsc helper methods are augmented to accept an extra hint to know
whether the code needs to be translated to ES5 format or not.

Fixes angular#32665
gkalpak added a commit to gkalpak/angular that referenced this issue Nov 4, 2019
Previously, ngcc's `Renderer` would add some constants in the processed
files which were emitted as ES2015 code (e.g. `const` declarations).
This would result in invalid ES5 generated code that would break when
run on browsers that do not support the emitted format.

This commit fixes it by adding a `printStatement()` method to
`RenderingFormatter`, which can convert statements to JavaScript code in
a suitable format for the corresponding `RenderingFormatter`.
Additionally, the `translateExpression()` and `translateStatement()`
ngtsc helper methods are augmented to accept an extra hint to know
whether the code needs to be translated to ES5 format or not.

Fixes angular#32665
gkalpak added a commit to gkalpak/angular that referenced this issue Nov 4, 2019
Previously, ngcc's `Renderer` would add some constants in the processed
files which were emitted as ES2015 code (e.g. `const` declarations).
This would result in invalid ES5 generated code that would break when
run on browsers that do not support the emitted format.

This commit fixes it by adding a `printStatement()` method to
`RenderingFormatter`, which can convert statements to JavaScript code in
a suitable format for the corresponding `RenderingFormatter`.
Additionally, the `translateExpression()` and `translateStatement()`
ngtsc helper methods are augmented to accept an extra hint to know
whether the code needs to be translated to ES5 format or not.

Fixes angular#32665
gkalpak added a commit to gkalpak/angular that referenced this issue Nov 5, 2019
Previously, ngcc's `Renderer` would add some constants in the processed
files which were emitted as ES2015 code (e.g. `const` declarations).
This would result in invalid ES5 generated code that would break when
run on browsers that do not support the emitted format.

This commit fixes it by adding a `printStatement()` method to
`RenderingFormatter`, which can convert statements to JavaScript code in
a suitable format for the corresponding `RenderingFormatter`.
Additionally, the `translateExpression()` and `translateStatement()`
ngtsc helper methods are augmented to accept an extra hint to know
whether the code needs to be translated to ES5 format or not.

Fixes angular#32665
gkalpak added a commit to gkalpak/angular that referenced this issue Nov 7, 2019
Previously, ngcc's `Renderer` would add some constants in the processed
files which were emitted as ES2015 code (e.g. `const` declarations).
This would result in invalid ES5 generated code that would break when
run on browsers that do not support the emitted format.

This commit fixes it by adding a `printStatement()` method to
`RenderingFormatter`, which can convert statements to JavaScript code in
a suitable format for the corresponding `RenderingFormatter`.
Additionally, the `translateExpression()` and `translateStatement()`
ngtsc helper methods are augmented to accept an extra hint to know
whether the code needs to be translated to ES5 format or not.

Fixes angular#32665
gkalpak added a commit to gkalpak/angular that referenced this issue Nov 7, 2019
Previously, ngcc's `Renderer` would add some constants in the processed
files which were emitted as ES2015 code (e.g. `const` declarations).
This would result in invalid ES5 generated code that would break when
run on browsers that do not support the emitted format.

This commit fixes it by adding a `printStatement()` method to
`RenderingFormatter`, which can convert statements to JavaScript code in
a suitable format for the corresponding `RenderingFormatter`.
Additionally, the `translateExpression()` and `translateStatement()`
ngtsc helper methods are augmented to accept an extra hint to know
whether the code needs to be translated to ES5 format or not.

Fixes angular#32665
@IgorMinar IgorMinar modified the milestones: Backlog, v9-blockers Nov 7, 2019
gkalpak added a commit to gkalpak/angular that referenced this issue Nov 13, 2019
Previously, ngcc's `Renderer` would add some constants in the processed
files which were emitted as ES2015 code (e.g. `const` declarations).
This would result in invalid ES5 generated code that would break when
run on browsers that do not support the emitted format.

This commit fixes it by adding a `printStatement()` method to
`RenderingFormatter`, which can convert statements to JavaScript code in
a suitable format for the corresponding `RenderingFormatter`.
Additionally, the `translateExpression()` and `translateStatement()`
ngtsc helper methods are augmented to accept an extra hint to know
whether the code needs to be translated to ES5 format or not.

Fixes angular#32665
gkalpak added a commit to gkalpak/angular that referenced this issue Nov 13, 2019
Previously, ngcc's `Renderer` would add some constants in the processed
files which were emitted as ES2015 code (e.g. `const` declarations).
This would result in invalid ES5 generated code that would break when
run on browsers that do not support the emitted format.

This commit fixes it by adding a `printStatement()` method to
`RenderingFormatter`, which can convert statements to JavaScript code in
a suitable format for the corresponding `RenderingFormatter`.
Additionally, the `translateExpression()` and `translateStatement()`
ngtsc helper methods are augmented to accept an extra hint to know
whether the code needs to be translated to ES5 format or not.

Fixes angular#32665
kara added a commit that referenced this issue Nov 13, 2019
Previously, ngcc's `Renderer` would add some constants in the processed
files which were emitted as ES2015 code (e.g. `const` declarations).
This would result in invalid ES5 generated code that would break when
run on browsers that do not support the emitted format.

This commit fixes it by adding a `printStatement()` method to
`RenderingFormatter`, which can convert statements to JavaScript code in
a suitable format for the corresponding `RenderingFormatter`.
Additionally, the `translateExpression()` and `translateStatement()`
ngtsc helper methods are augmented to accept an extra hint to know
whether the code needs to be translated to ES5 format or not.

Fixes #32665

PR Close #33514
@kara kara closed this in 033aba9 Nov 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can鈥檛 perform that action at this time.