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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tree-shakable providers IE10 #24339

Closed
meiordac opened this issue Jun 7, 2018 · 12 comments
Closed

Tree-shakable providers IE10 #24339

meiordac opened this issue Jun 7, 2018 · 12 comments
Labels
area: core Issues related to the framework runtime freq2: medium regression Indicates than the issue relates to something that worked in a previous version type: bug/fix
Milestone

Comments

@meiordac
Copy link

meiordac commented Jun 7, 2018

I'm submitting a...


[ ] Regression (a behavior that used to work and stopped working in a new release)
[x] Bug report  
[ ] Performance issue
[ ] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead see https://github.com/angular/angular/blob/master/CONTRIBUTING.md#question
[ ] Other... Please describe:

Current behavior

After upgrading to Angular 6 from Angular 5 I tried to implement tree-shakable services but
IE10 is giving me the following error: Uncaught (in promise): TypeError: Unable to get property 'ngMetadataName' of undefined or null reference

Expected behavior

Tree-shakable services work without errors in IE10.

Minimal reproduction of the problem with instructions

https://angular.io/generated/live-examples/toh-pt6/stackblitz.html

Stackblitz does not seem to work in IE10 either.

What is the motivation / use case for changing the behavior?

Use tree shakable components including IE10

Environment


Angular version: 6.0.3
Angular CLI: 6.0.7

Browser:
- [ ] Chrome (desktop) version XX
- [ ] Chrome (Android) version XX
- [ ] Chrome (iOS) version XX
- [ ] Firefox version XX
- [ ] Safari (desktop) version XX
- [ ] Safari (iOS) version XX
- [x ] IE version 10
- [ ] Edge version XX
 
For Tooling issues:
- Node version: v8.11.1
- Platform:  MacOS

Others:
Tested using BrowserStack on Windows 8 running IE10 latest.

@mrkapocs
Copy link

mrkapocs commented Jun 7, 2018

Same thing happens in my project with IE10

@mhevery mhevery added the area: core Issues related to the framework runtime label Jun 11, 2018
@ngbot ngbot bot added this to the needsTriage milestone Jun 11, 2018
@mrkapocs
Copy link

I got my app working by doing the next.

In angular 6 the cli generated services are created like:
@Injectable({
providedIn: 'root'
})
export class MyService{ ... }

After removing the object parameter from the Injectable marker, and adding the service to the providers array of the module it is used in, this bug is not showing anymore.

So the service looks like this:
@Injectable()
export class MyService{ ... }

@beyerleinf
Copy link

@mrkapocs That works but it's a workaround and not really desired behavior imho.

@mrkapocs
Copy link

@beyerleinf Yeah, I know. This is still a serious problem, and my solution is just a workaround for those who need it.

@mhevery mhevery added regression Indicates than the issue relates to something that worked in a previous version freq2: medium type: bug/fix labels Jun 26, 2018
@ngbot ngbot bot modified the milestones: needsTriage, Backlog Jun 26, 2018
@alxhub
Copy link
Member

alxhub commented Jun 26, 2018

In the StackBlitz polyfills.ts:

/** IE10 and IE11 requires the following for the Reflect API. */
// import 'core-js/es6/reflect';


/** Evergreen browsers require these. **/
// Used for reflect-metadata in JIT. If you use AOT (and only Angular decorators), you can remove.
import 'core-js/es7/reflect';

Does it work if you use the reflect polyfill that's specified for IE9?

@beyerleinf
Copy link

@alxhub I created this StackBlitz https://stackblitz.com/edit/angular-9ecstk and didn't modify anything. Does this have the import you mean?

As you can see in this image, it doesn't work in IE10

@trotyl
Copy link
Contributor

trotyl commented Jun 27, 2018

@beyerleinf StackBlitz depends on Firebase which only supports IE11+, that issue has nothing to do with Angular, it's impossible to reproduce IE compatible problem via StackBlitz.

@trotyl
Copy link
Contributor

trotyl commented Jun 27, 2018

@meiordac I'm unable to reproduce the problem on downloaded official tutorial project (with polyfills enabled):

image

Could you please create a new Angular CLI project on GitHub to reproduce it?

@beyerleinf
Copy link

@trotyl My bad

@maxokorokov
Copy link

maxokorokov commented Jun 28, 2018

Looks like there is a problem in IE10 in dev mode when there are transitive injections.
AOT works fine. Both with Angular CLI 6.0.8 (all polyfills on) and custom build with core-js/shim

Simple use case:

@Injectable() // ← doesn't matter where it is provided
export class B {
}

@Injectable({providedIn: 'root'})
export class A {
  constructor(b: B) {}
}

@Component({ ... })
export class App {
  constructor(a: A) {}
}

Same thing if you replace B with injection token

@alxhub
Copy link
Member

alxhub commented Jun 28, 2018

I'm happy to report that this was fixed in e3759f7. The fix is in 6.1 beta currently, and will be released in 6.1. The bug only affects JIT mode, so as a workaround until then you can test your apps with AOT in IE10.

@angular-automatic-lock-bot
Copy link

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.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: core Issues related to the framework runtime freq2: medium regression Indicates than the issue relates to something that worked in a previous version type: bug/fix
Projects
None yet
Development

No branches or pull requests

7 participants