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

ngcc: handle localized messages in ES5 #33857

Conversation

@petebacondarwin
Copy link
Member

petebacondarwin commented Nov 15, 2019

Recently the ngtsc translator was modified to be more ScriptTarget
aware, which basically means that it will not generate non-ES5 code
when the output format is ES5 or similar.

This commit enhances that change by also "downleveling" localized
messages. In ES2015 the messages use tagged template literals, which
are not available in ES5.

@ngbot ngbot bot modified the milestone: needsTriage Nov 15, 2019
@googlebot googlebot added the cla: yes label Nov 15, 2019
@petebacondarwin petebacondarwin force-pushed the petebacondarwin:ngcc-es5-localize-strings branch from c64f167 to bc1f6ad Nov 15, 2019
@petebacondarwin petebacondarwin marked this pull request as ready for review Nov 15, 2019
@petebacondarwin petebacondarwin requested review from angular/fw-compiler as code owners Nov 15, 2019
@petebacondarwin petebacondarwin force-pushed the petebacondarwin:ngcc-es5-localize-strings branch 2 times, most recently from 0a6e715 to 6c3285e Nov 15, 2019
@@ -458,7 +454,7 @@ export class TypeTranslatorVisitor implements ExpressionVisitor, TypeVisitor {
}

visitLocalizedString(ast: LocalizedString, context: Context): ts.Expression {
return visitLocalizedString(ast, context, this);
throw new Error('Method not implemented.');

This comment has been minimized.

Copy link
@petebacondarwin

petebacondarwin Nov 15, 2019

Author Member

I realised that we never actually call this because it is in a visitor that only cares about types.

@petebacondarwin petebacondarwin changed the title ngcc: handler localized messages in ES5 ngcc: handle localized messages in ES5 Nov 15, 2019
@petebacondarwin petebacondarwin force-pushed the petebacondarwin:ngcc-es5-localize-strings branch 2 times, most recently from 4c4961b to 617efb0 Nov 15, 2019
@petebacondarwin petebacondarwin force-pushed the petebacondarwin:ngcc-es5-localize-strings branch from 617efb0 to 81294f8 Nov 19, 2019
@petebacondarwin petebacondarwin removed request for angular/fw-i18n Nov 19, 2019
@alxhub
alxhub approved these changes Nov 19, 2019
Copy link
Contributor

alxhub left a comment

LGTM with two comments.

@petebacondarwin petebacondarwin force-pushed the petebacondarwin:ngcc-es5-localize-strings branch from 81294f8 to 5db61f6 Nov 20, 2019
Recently the ngtsc translator was modified to be more `ScriptTarget`
aware, which basically means that it will not generate non-ES5 code
when the output format is ES5 or similar.

This commit enhances that change by also "downleveling" localized
messages. In ES2015 the messages use tagged template literals, which
are not available in ES5.
@petebacondarwin petebacondarwin force-pushed the petebacondarwin:ngcc-es5-localize-strings branch from 5db61f6 to 05aba08 Nov 21, 2019
@petebacondarwin petebacondarwin force-pushed the petebacondarwin:ngcc-es5-localize-strings branch from 05aba08 to 770a504 Nov 21, 2019
@alxhub

This comment has been minimized.

Copy link
Contributor

alxhub commented Nov 21, 2019

alxhub added a commit that referenced this pull request Nov 21, 2019
Recently the ngtsc translator was modified to be more `ScriptTarget`
aware, which basically means that it will not generate non-ES5 code
when the output format is ES5 or similar.

This commit enhances that change by also "downleveling" localized
messages. In ES2015 the messages use tagged template literals, which
are not available in ES5.

PR Close #33857
@alxhub alxhub closed this in bf1bcd1 Nov 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.