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

fix(common): do not override locale provided on bootstrap #13654

Merged
merged 1 commit into from
Jan 5, 2017

Conversation

DzmitryShylovich
Copy link
Contributor

Closes #13607

Copy link
Contributor

@vicb vicb left a comment

Choose a reason for hiding this comment

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

Please add a test

@vicb vicb added area: common Issues related to APIs in the @angular/common package action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews pr_state: LGTM labels Dec 27, 2016
@DzmitryShylovich
Copy link
Contributor Author

DzmitryShylovich commented Dec 28, 2016

@vicb There's a problem with writing a test case for it :)
I need to create a new platform with providers, but there's already a default platform configured for test thus I cannot create a new one (only 1 platform can be active at a time). I also cannot destry a default one because, for some reason, it's already marked as destroyed. I faced the same issue here #13105
I tried to use compiler providers:

defaultPlatform.bootstrapModule(createModule(), {providers: [{provide: LOCALE_ID, useValue: 'fr-FR'}]})

but they are not available for the app.
So do u know where I can found a test example with a custom platform or do u have an idea how I can test it without creating a new platform?

@vicb
Copy link
Contributor

vicb commented Dec 28, 2016

Create an e2e test?

@DzmitryShylovich
Copy link
Contributor Author

@vicb added a test

@vicb vicb added action: merge The PR is ready for merge by the caretaker and removed action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews labels Dec 29, 2016
@vicb
Copy link
Contributor

vicb commented Dec 29, 2016

great, thanks !

@IgorMinar IgorMinar merged commit 2dd6280 into angular:master Jan 5, 2017
@DzmitryShylovich DzmitryShylovich deleted the gh/13607 branch January 5, 2017 17:25
IgorMinar pushed a commit to IgorMinar/angular that referenced this pull request Jan 6, 2017
@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 10, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker area: common Issues related to APIs in the @angular/common package cla: yes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LOCALE_ID is overwritten with platformBrowserDynamic
4 participants