Skip to content

Conversation

mprobst
Copy link
Contributor

@mprobst mprobst commented May 4, 2017

This code only runs in ES5 mode in the test suite, so this is difficult to test. However updateFromTemplate is being called with a spread operator, as ...updateFromTemplate(...). The spread operator should fail on null values. This change avoids the problem by always returning a (possibly empty) array.

@mprobst mprobst added area: i18n Issues related to localization and internationalization hotlist: google type: bug/fix labels May 4, 2017
@mprobst mprobst requested review from vicb and ocombe May 4, 2017 17:37
This code only runs in ES5 mode in the test suite, so this is difficult to test.
However `updateFromTemplate` is being called with a spread operator, as
`...updateFromTemplate(...)`. The spread operator should fail on `null` values.
This change avoids the problem by always returning a (possibly empty) array.
Copy link
Contributor

@ocombe ocombe left a comment

Choose a reason for hiding this comment

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

This makes sense, I don't any side effect to it

@mprobst mprobst added the action: merge The PR is ready for merge by the caretaker label May 4, 2017
matsko pushed a commit that referenced this pull request May 4, 2017
This code only runs in ES5 mode in the test suite, so this is difficult to test.
However `updateFromTemplate` is being called with a spread operator, as
`...updateFromTemplate(...)`. The spread operator should fail on `null` values.
This change avoids the problem by always returning a (possibly empty) array.

PR Close #16547
@mprobst
Copy link
Contributor Author

mprobst commented May 5, 2017

@matsko I'm confused - was this merged?

@mhevery mhevery closed this in e0a8376 May 8, 2017
jasonaden pushed a commit to jasonaden/angular that referenced this pull request May 10, 2017
This code only runs in ES5 mode in the test suite, so this is difficult to test.
However `updateFromTemplate` is being called with a spread operator, as
`...updateFromTemplate(...)`. The spread operator should fail on `null` values.
This change avoids the problem by always returning a (possibly empty) array.

PR Close angular#16547
@smoke
Copy link
Contributor

smoke commented May 11, 2017

I am not quite sure, but this have probably fixed an issue with i18n I was having with angular 4.1.1 where given:

<h1 i18n>Some text with interpolated stuff here {{title}}</h1>

after AOT build with provided translation files rendered empty tags like

<h1></h1>

@mprobst
Copy link
Contributor Author

mprobst commented May 11, 2017 via email

@mhevery mhevery deleted the mprobst-spread branch July 27, 2017 22:11
asnowwolf pushed a commit to asnowwolf/angular that referenced this pull request Aug 11, 2017
This code only runs in ES5 mode in the test suite, so this is difficult to test.
However `updateFromTemplate` is being called with a spread operator, as
`...updateFromTemplate(...)`. The spread operator should fail on `null` values.
This change avoids the problem by always returning a (possibly empty) array.

PR Close angular#16547
juleskremer pushed a commit to juleskremer/angular that referenced this pull request Aug 28, 2017
This code only runs in ES5 mode in the test suite, so this is difficult to test.
However `updateFromTemplate` is being called with a spread operator, as
`...updateFromTemplate(...)`. The spread operator should fail on `null` values.
This change avoids the problem by always returning a (possibly empty) array.

PR Close angular#16547
@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 12, 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: i18n Issues related to localization and internationalization cla: yes hotlist: google type: bug/fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants