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 generates invalid property metadata #30569

Closed
devversion opened this issue May 20, 2019 · 12 comments

Comments

@devversion
Copy link
Member

@devversion devversion commented May 20, 2019

馃悶 bug report

Affected Package

@angular/compiler-cli / ngcc

Description

Currently when compiling @angular/cdk/a11y with ngcc, the generated property metadata call (setClassMetadata) incorrectly sets metadata for the autoCapture property within the FocusTrap twice.

ngcc0.傻setClassMetadata(..., {
  autoCapture: [{
      type: Input,
      args: ['cdkTrapFocusAutoCapture']
    }], 
  autoCapture: [], 
});

This means that the first occurrence will be accidentally overwritten and the metadata is discarded. The duplication seems to happen because autoCapture declares a getter and a setter.

As far as I could tell the issue is that ngcc returns the getter and setter for reflection.getMembersOfClass(FocusTrap) but sets decorators that are either defined on getter or setter as decorators for both members. This then results in two properties with the same name in the setClassMetadata call.

https://github.com/angular/angular/blob/master/packages/compiler-cli/src/ngtsc/annotations/src/metadata.ts#L59-L62

馃敩 Minimal Reproduction

https://github.com/devversion/ivy-ngcc-generates-duplicate-prop-metadata

馃敟 Exception or Error

See: angular/components#16066

Defining the same property multiple times seems to cause an exception in IE11 with use strict mode. This is actually reasonable as the first declaration is accidentally overwritten in an object literal.

馃實 Your Environment

Angular Version:


@angular/*: 8.0.0-rc.4
@angular/cdk: 8.0.0-rc.1
@ngbot ngbot bot added this to the needsTriage milestone May 20, 2019
@devversion devversion changed the title Ngcc generates invalid property metadata static property ngcc generates invalid property metadata May 20, 2019
@ngbot ngbot bot modified the milestones: needsTriage, Backlog May 23, 2019
@ajosef

This comment has been minimized.

Copy link

@ajosef ajosef commented Jul 11, 2019

#30637 is closed, which was marked as a duplicate of this issue, but Ivy is still not working on IE11 even in Angular 8.1.
Is there any way to make Ivy work on IE11?

@rajeshmogasala

This comment has been minimized.

Copy link

@rajeshmogasala rajeshmogasala commented Jul 19, 2019

Same problem for me also.Could you please tell us is it fixed or do we have any work around ?
Thank you.
SCRIPT1046: Multiple definitions of a property not allowed in strict mode

@preli

This comment has been minimized.

Copy link

@preli preli commented Aug 6, 2019

Same for me

@filipesilva

This comment has been minimized.

Copy link
Member

@filipesilva filipesilva commented Aug 19, 2019

I see this happening in Angular CLI 9.0.0-next.0 apps using IE10:

SCRIPT1046: Multiple definitions of a property not allowed in strict mode
/*@__PURE__*/ _angular_core__WEBPACK_IMPORTED_MODULE_0__["傻setClassMetadata"](NgForOf, [{
        type: _angular_core__WEBPACK_IMPORTED_MODULE_0__["Directive"],
        args: [{ selector: '[ngFor][ngForOf]' }]
    }], function () { return [{ type: _angular_core__WEBPACK_IMPORTED_MODULE_0__["ViewContainerRef"] }, { type: _angular_core__WEBPACK_IMPORTED_MODULE_0__["TemplateRef"] }, { type: _angular_core__WEBPACK_IMPORTED_MODULE_0__["IterableDiffers"] }]; }, { _viewContainer: [], _template: [], _differs: [], _ngForOfDirty: [], _differ: [], ngForOf: [{
            type: _angular_core__WEBPACK_IMPORTED_MODULE_0__["Input"]
        }], ngForTrackBy: [{
            type: _angular_core__WEBPACK_IMPORTED_MODULE_0__["Input"]
        }], ngForTrackBy: [], ngForTemplate: [{
            type: _angular_core__WEBPACK_IMPORTED_MODULE_0__["Input"]
        }], ngDoCheck: [], _applyChanges: [], _perViewChange: [] });
    return NgForOf;
}());
@richardbrammer

This comment has been minimized.

Copy link

@richardbrammer richardbrammer commented Aug 21, 2019

I get the same error using Ivy and IE11 in vendor-es5.js

@Splaktar

This comment has been minimized.

Copy link
Member

@Splaktar Splaktar commented Sep 18, 2019

Is freq1: low correct for this issue? It seems a little low for an issue that breaks apps on an entire, supported, browser?

@gkalpak gkalpak added freq3: high and removed freq1: low labels Sep 23, 2019
@matzematthaey

This comment has been minimized.

Copy link

@matzematthaey matzematthaey commented Sep 30, 2019

same issue for us. blocks using angular8+ in IE11.

@gkalpak

This comment has been minimized.

Copy link
Member

@gkalpak gkalpak commented Sep 30, 2019

@matzematthaey: Just to clarify, this issue if about ngcc, which means that it only affects app with Ivy turned on (which is the default in v9, but not v8).

@SpeedoPasanen

This comment has been minimized.

Copy link

@SpeedoPasanen SpeedoPasanen commented Sep 30, 2019

In case someone needs to push an IVY app to production now (not sure if it's a good idea) and support IE, a workaround is to search "use strict"; in dist/*-es5* and replace all occurrances with empty.

@james-schwartzkopf

This comment has been minimized.

Copy link

@james-schwartzkopf james-schwartzkopf commented Sep 30, 2019

In case someone needs to push an IVY app to production now (not sure if it's a good idea) and support IE, a workaround is to search "use strict"; in dist/*-es5* and replace all occurrances with empty.

Better to replace it with '"----------"; so source maps, etc. still line up.

This is the script we are using:

import * as fs from 'fs';
import * as path from 'path';

//TODO get rid of this once @angular/angular #30569 is fixed
//https://github.com/angular/angular/issues/30569

const [_, __, ...dirs] = process.argv;

dirs.forEach(
  dir => fs.readdirSync(dir)
    .filter(filename => /[.]js$/.test(filename))
    .filter(filename => !/-es2015(|[.][0-9a-f]+)[.]js$/.test(filename))
    .forEach(filename => {
      const filepath = path.join(dir, filename);
      const text = fs.readFileSync(filepath, 'utf-8');
      const newText = text.replace(/"use strict";/g, '"----------";');
      if (newText !== text) {
        fs.writeFileSync(filepath, newText, 'utf-8');
      }
    })
);
@gkalpak gkalpak self-assigned this Oct 25, 2019
gkalpak added a commit to gkalpak/angular that referenced this issue Oct 30, 2019
鈥operties

While processing class metadata, ngtsc generates a `setClassMetadata()`
call which (among other things) contains info about property decorators.
Previously, processing getter/setter pairs with some of ngcc's
`ReflectionHost`s resulted in multiple metadata entries for the same
property, which resulted in duplicate object keys, which in turn causes
an error in ES5 strict mode.

This commit fixes it by ensuring that there are no duplicate property
names in the `setClassMetadata()` calls.

In addition, `generateSetClassMetadataCall()` is updated to treat
`ClassMember#decorators: []` the same as `ClassMember.decorators: null`
(i.e. omitting the `ClassMember` from the generated `setClassMetadata()`
call). Alternatively, ngcc's `ReflectionHost`s could be updated to do
this transformation (`decorators: []` --> `decorators: null`) when
reflecting on class members, but this would require changes in many
places and be less future-proof.

For example, given a class such as:

```ts
class Foo {
  @input() get bar() { return 'bar'; }
  set bar(value: any) {}
}
```

...previously the generated `setClassMetadata()` call would look like:

```ts
傻setClassMetadata(..., {
  bar: [{type: Input}],
  bar: [],
});
```

The same class will now result in a call like:

```ts
傻setClassMetadata(..., {
  bar: [{type: Input}],
});
```

Fixes angular#30569
gkalpak added a commit to gkalpak/angular that referenced this issue Oct 30, 2019
鈥operties

While processing class metadata, ngtsc generates a `setClassMetadata()`
call which (among other things) contains info about property decorators.
Previously, processing getter/setter pairs with some of ngcc's
`ReflectionHost`s resulted in multiple metadata entries for the same
property, which resulted in duplicate object keys, which in turn causes
an error in ES5 strict mode.

This commit fixes it by ensuring that there are no duplicate property
names in the `setClassMetadata()` calls.

In addition, `generateSetClassMetadataCall()` is updated to treat
`ClassMember#decorators: []` the same as `ClassMember.decorators: null`
(i.e. omitting the `ClassMember` from the generated `setClassMetadata()`
call). Alternatively, ngcc's `ReflectionHost`s could be updated to do
this transformation (`decorators: []` --> `decorators: null`) when
reflecting on class members, but this would require changes in many
places and be less future-proof.

For example, given a class such as:

```ts
class Foo {
  @input() get bar() { return 'bar'; }
  set bar(value: any) {}
}
```

...previously the generated `setClassMetadata()` call would look like:

```ts
傻setClassMetadata(..., {
  bar: [{type: Input}],
  bar: [],
});
```

The same class will now result in a call like:

```ts
傻setClassMetadata(..., {
  bar: [{type: Input}],
});
```

Fixes angular#30569
gkalpak added a commit to gkalpak/angular that referenced this issue Oct 31, 2019
鈥operties

While processing class metadata, ngtsc generates a `setClassMetadata()`
call which (among other things) contains info about property decorators.
Previously, processing getter/setter pairs with some of ngcc's
`ReflectionHost`s resulted in multiple metadata entries for the same
property, which resulted in duplicate object keys, which in turn causes
an error in ES5 strict mode.

This commit fixes it by ensuring that there are no duplicate property
names in the `setClassMetadata()` calls.

In addition, `generateSetClassMetadataCall()` is updated to treat
`ClassMember#decorators: []` the same as `ClassMember.decorators: null`
(i.e. omitting the `ClassMember` from the generated `setClassMetadata()`
call). Alternatively, ngcc's `ReflectionHost`s could be updated to do
this transformation (`decorators: []` --> `decorators: null`) when
reflecting on class members, but this would require changes in many
places and be less future-proof.

For example, given a class such as:

```ts
class Foo {
  @input() get bar() { return 'bar'; }
  set bar(value: any) {}
}
```

...previously the generated `setClassMetadata()` call would look like:

```ts
傻setClassMetadata(..., {
  bar: [{type: Input}],
  bar: [],
});
```

The same class will now result in a call like:

```ts
傻setClassMetadata(..., {
  bar: [{type: Input}],
});
```

Fixes angular#30569
gkalpak added a commit to gkalpak/angular that referenced this issue Oct 31, 2019
鈥operties

While processing class metadata, ngtsc generates a `setClassMetadata()`
call which (among other things) contains info about property decorators.
Previously, processing getter/setter pairs with some of ngcc's
`ReflectionHost`s resulted in multiple metadata entries for the same
property, which resulted in duplicate object keys, which in turn causes
an error in ES5 strict mode.

This commit fixes it by ensuring that there are no duplicate property
names in the `setClassMetadata()` calls.

In addition, `generateSetClassMetadataCall()` is updated to treat
`ClassMember#decorators: []` the same as `ClassMember.decorators: null`
(i.e. omitting the `ClassMember` from the generated `setClassMetadata()`
call). Alternatively, ngcc's `ReflectionHost`s could be updated to do
this transformation (`decorators: []` --> `decorators: null`) when
reflecting on class members, but this would require changes in many
places and be less future-proof.

For example, given a class such as:

```ts
class Foo {
  @input() get bar() { return 'bar'; }
  set bar(value: any) {}
}
```

...previously the generated `setClassMetadata()` call would look like:

```ts
傻setClassMetadata(..., {
  bar: [{type: Input}],
  bar: [],
});
```

The same class will now result in a call like:

```ts
傻setClassMetadata(..., {
  bar: [{type: Input}],
});
```

Fixes angular#30569
@maxokorokov maxokorokov mentioned this issue Nov 1, 2019
7 of 10 tasks complete
@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
鈥operties

While processing class metadata, ngtsc generates a `setClassMetadata()`
call which (among other things) contains info about property decorators.
Previously, processing getter/setter pairs with some of ngcc's
`ReflectionHost`s resulted in multiple metadata entries for the same
property, which resulted in duplicate object keys, which in turn causes
an error in ES5 strict mode.

This commit fixes it by ensuring that there are no duplicate property
names in the `setClassMetadata()` calls.

In addition, `generateSetClassMetadataCall()` is updated to treat
`ClassMember#decorators: []` the same as `ClassMember.decorators: null`
(i.e. omitting the `ClassMember` from the generated `setClassMetadata()`
call). Alternatively, ngcc's `ReflectionHost`s could be updated to do
this transformation (`decorators: []` --> `decorators: null`) when
reflecting on class members, but this would require changes in many
places and be less future-proof.

For example, given a class such as:

```ts
class Foo {
  @input() get bar() { return 'bar'; }
  set bar(value: any) {}
}
```

...previously the generated `setClassMetadata()` call would look like:

```ts
傻setClassMetadata(..., {
  bar: [{type: Input}],
  bar: [],
});
```

The same class will now result in a call like:

```ts
傻setClassMetadata(..., {
  bar: [{type: Input}],
});
```

Fixes angular#30569
gkalpak added a commit to gkalpak/angular that referenced this issue Nov 4, 2019
鈥operties

While processing class metadata, ngtsc generates a `setClassMetadata()`
call which (among other things) contains info about property decorators.
Previously, processing getter/setter pairs with some of ngcc's
`ReflectionHost`s resulted in multiple metadata entries for the same
property, which resulted in duplicate object keys, which in turn causes
an error in ES5 strict mode.

This commit fixes it by ensuring that there are no duplicate property
names in the `setClassMetadata()` calls.

In addition, `generateSetClassMetadataCall()` is updated to treat
`ClassMember#decorators: []` the same as `ClassMember.decorators: null`
(i.e. omitting the `ClassMember` from the generated `setClassMetadata()`
call). Alternatively, ngcc's `ReflectionHost`s could be updated to do
this transformation (`decorators: []` --> `decorators: null`) when
reflecting on class members, but this would require changes in many
places and be less future-proof.

For example, given a class such as:

```ts
class Foo {
  @input() get bar() { return 'bar'; }
  set bar(value: any) {}
}
```

...previously the generated `setClassMetadata()` call would look like:

```ts
傻setClassMetadata(..., {
  bar: [{type: Input}],
  bar: [],
});
```

The same class will now result in a call like:

```ts
傻setClassMetadata(..., {
  bar: [{type: Input}],
});
```

Fixes angular#30569
gkalpak added a commit to gkalpak/angular that referenced this issue Nov 5, 2019
鈥operties

While processing class metadata, ngtsc generates a `setClassMetadata()`
call which (among other things) contains info about property decorators.
Previously, processing getter/setter pairs with some of ngcc's
`ReflectionHost`s resulted in multiple metadata entries for the same
property, which resulted in duplicate object keys, which in turn causes
an error in ES5 strict mode.

This commit fixes it by ensuring that there are no duplicate property
names in the `setClassMetadata()` calls.

In addition, `generateSetClassMetadataCall()` is updated to treat
`ClassMember#decorators: []` the same as `ClassMember.decorators: null`
(i.e. omitting the `ClassMember` from the generated `setClassMetadata()`
call). Alternatively, ngcc's `ReflectionHost`s could be updated to do
this transformation (`decorators: []` --> `decorators: null`) when
reflecting on class members, but this would require changes in many
places and be less future-proof.

For example, given a class such as:

```ts
class Foo {
  @input() get bar() { return 'bar'; }
  set bar(value: any) {}
}
```

...previously the generated `setClassMetadata()` call would look like:

```ts
傻setClassMetadata(..., {
  bar: [{type: Input}],
  bar: [],
});
```

The same class will now result in a call like:

```ts
傻setClassMetadata(..., {
  bar: [{type: Input}],
});
```

Fixes angular#30569
gkalpak added a commit to gkalpak/angular that referenced this issue Nov 7, 2019
鈥operties

While processing class metadata, ngtsc generates a `setClassMetadata()`
call which (among other things) contains info about property decorators.
Previously, processing getter/setter pairs with some of ngcc's
`ReflectionHost`s resulted in multiple metadata entries for the same
property, which resulted in duplicate object keys, which in turn causes
an error in ES5 strict mode.

This commit fixes it by ensuring that there are no duplicate property
names in the `setClassMetadata()` calls.

In addition, `generateSetClassMetadataCall()` is updated to treat
`ClassMember#decorators: []` the same as `ClassMember.decorators: null`
(i.e. omitting the `ClassMember` from the generated `setClassMetadata()`
call). Alternatively, ngcc's `ReflectionHost`s could be updated to do
this transformation (`decorators: []` --> `decorators: null`) when
reflecting on class members, but this would require changes in many
places and be less future-proof.

For example, given a class such as:

```ts
class Foo {
  @input() get bar() { return 'bar'; }
  set bar(value: any) {}
}
```

...previously the generated `setClassMetadata()` call would look like:

```ts
傻setClassMetadata(..., {
  bar: [{type: Input}],
  bar: [],
});
```

The same class will now result in a call like:

```ts
傻setClassMetadata(..., {
  bar: [{type: Input}],
});
```

Fixes angular#30569
@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
鈥operties

While processing class metadata, ngtsc generates a `setClassMetadata()`
call which (among other things) contains info about property decorators.
Previously, processing getter/setter pairs with some of ngcc's
`ReflectionHost`s resulted in multiple metadata entries for the same
property, which resulted in duplicate object keys, which in turn causes
an error in ES5 strict mode.

This commit fixes it by ensuring that there are no duplicate property
names in the `setClassMetadata()` calls.

In addition, `generateSetClassMetadataCall()` is updated to treat
`ClassMember#decorators: []` the same as `ClassMember.decorators: null`
(i.e. omitting the `ClassMember` from the generated `setClassMetadata()`
call). Alternatively, ngcc's `ReflectionHost`s could be updated to do
this transformation (`decorators: []` --> `decorators: null`) when
reflecting on class members, but this would require changes in many
places and be less future-proof.

For example, given a class such as:

```ts
class Foo {
  @input() get bar() { return 'bar'; }
  set bar(value: any) {}
}
```

...previously the generated `setClassMetadata()` call would look like:

```ts
傻setClassMetadata(..., {
  bar: [{type: Input}],
  bar: [],
});
```

The same class will now result in a call like:

```ts
傻setClassMetadata(..., {
  bar: [{type: Input}],
});
```

Fixes angular#30569
gkalpak added a commit to gkalpak/angular that referenced this issue Nov 13, 2019
鈥operties

While processing class metadata, ngtsc generates a `setClassMetadata()`
call which (among other things) contains info about property decorators.
Previously, processing getter/setter pairs with some of ngcc's
`ReflectionHost`s resulted in multiple metadata entries for the same
property, which resulted in duplicate object keys, which in turn causes
an error in ES5 strict mode.

This commit fixes it by ensuring that there are no duplicate property
names in the `setClassMetadata()` calls.

In addition, `generateSetClassMetadataCall()` is updated to treat
`ClassMember#decorators: []` the same as `ClassMember.decorators: null`
(i.e. omitting the `ClassMember` from the generated `setClassMetadata()`
call). Alternatively, ngcc's `ReflectionHost`s could be updated to do
this transformation (`decorators: []` --> `decorators: null`) when
reflecting on class members, but this would require changes in many
places and be less future-proof.

For example, given a class such as:

```ts
class Foo {
  @input() get bar() { return 'bar'; }
  set bar(value: any) {}
}
```

...previously the generated `setClassMetadata()` call would look like:

```ts
傻setClassMetadata(..., {
  bar: [{type: Input}],
  bar: [],
});
```

The same class will now result in a call like:

```ts
傻setClassMetadata(..., {
  bar: [{type: Input}],
});
```

Fixes angular#30569
kara added a commit that referenced this issue Nov 13, 2019
鈥operties (#33514)

While processing class metadata, ngtsc generates a `setClassMetadata()`
call which (among other things) contains info about property decorators.
Previously, processing getter/setter pairs with some of ngcc's
`ReflectionHost`s resulted in multiple metadata entries for the same
property, which resulted in duplicate object keys, which in turn causes
an error in ES5 strict mode.

This commit fixes it by ensuring that there are no duplicate property
names in the `setClassMetadata()` calls.

In addition, `generateSetClassMetadataCall()` is updated to treat
`ClassMember#decorators: []` the same as `ClassMember.decorators: null`
(i.e. omitting the `ClassMember` from the generated `setClassMetadata()`
call). Alternatively, ngcc's `ReflectionHost`s could be updated to do
this transformation (`decorators: []` --> `decorators: null`) when
reflecting on class members, but this would require changes in many
places and be less future-proof.

For example, given a class such as:

```ts
class Foo {
  @input() get bar() { return 'bar'; }
  set bar(value: any) {}
}
```

...previously the generated `setClassMetadata()` call would look like:

```ts
傻setClassMetadata(..., {
  bar: [{type: Input}],
  bar: [],
});
```

The same class will now result in a call like:

```ts
傻setClassMetadata(..., {
  bar: [{type: Input}],
});
```

Fixes #30569

PR Close #33514
@kara kara closed this in 7047751 Nov 13, 2019
@nmainardi

This comment has been minimized.

Copy link

@nmainardi nmainardi commented Dec 2, 2019

this problem still persists with this configuration:

Angular CLI: 8.3.20
Node: 13.2.0
OS: win32 x64
Angular: 8.2.14
... animations, common, compiler, compiler-cli, core, forms
... language-service, platform-browser, platform-browser-dynamic
... router

Package                           Version
-----------------------------------------------------------
@angular-devkit/architect         0.803.20
@angular-devkit/build-angular     0.803.20
@angular-devkit/build-optimizer   0.803.20
@angular-devkit/build-webpack     0.803.20
@angular-devkit/core              8.3.20
@angular-devkit/schematics        8.3.20
@angular/cli                      8.3.20
@ngtools/webpack                  8.3.20
@schematics/angular               8.3.20
@schematics/update                0.803.20
rxjs                              6.4.0
typescript                        3.5.3
webpack                           4.39.2

If you need my test project I'm going to upload it tomorrow.
@james-schwartzkopf how to implement your workaround? How are you using your script? Thank you all

@M4R1KU

This comment has been minimized.

Copy link

@M4R1KU M4R1KU commented Dec 2, 2019

@nmainardi It is fixed in 9.0.0-rc.3. You have to upgrade to atleast this version.

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.