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

feat(compiler): set `enableLegacyTemplate` to false by default #18756

Closed
wants to merge 1 commit into from

Conversation

@ocombe
Copy link
Contributor

ocombe commented Aug 17, 2017

PR Type

What kind of change does this PR introduce?

[x] Feature

What is the current behavior?

The compiler option enableLegacyTemplate is enabled by default

What is the new behavior?

The compiler option enableLegacyTemplate is now disabled by default as <template> elements have been deprecated since v4. Use <ng-template> instead.

Does this PR introduce a breaking change?

[x] Yes
@vicb

vicb approved these changes Aug 17, 2017

Copy link
Contributor

vicb left a comment

Please add a note in the commit message that this seeting will be removed in v6.

@ocombe ocombe force-pushed the ocombe:disable-legacy-template branch 2 times, most recently from 63e34a7 to 1cf7c68 Aug 18, 2017

@angular angular deleted a comment from mary-poppins Aug 18, 2017

@angular angular deleted a comment from mary-poppins Aug 18, 2017

@ocombe ocombe force-pushed the ocombe:disable-legacy-template branch from 1cf7c68 to eaaf4c2 Aug 22, 2017

@angular angular deleted a comment from mary-poppins Aug 22, 2017

@ocombe ocombe force-pushed the ocombe:disable-legacy-template branch from eaaf4c2 to e255017 Aug 22, 2017

@angular angular deleted a comment from mary-poppins Aug 22, 2017

@ocombe

This comment has been minimized.

Copy link
Contributor Author

ocombe commented Aug 22, 2017

@juleskremer can you validate the changes in the structural directives guide/examples please? https://pr18756-72287eb.ngbuilds.io/guide/structural-directives

@ocombe ocombe force-pushed the ocombe:disable-legacy-template branch from e255017 to 72287eb Aug 22, 2017

@mary-poppins

This comment has been minimized.

Copy link

mary-poppins commented Aug 22, 2017

@angular angular deleted a comment from mary-poppins Aug 22, 2017

@@ -105,9 +105,9 @@
The <code>hero.id</code> in the &lt;template [ngIf]&gt;

This comment has been minimized.

Copy link
@vicb

vicb Aug 22, 2017

Contributor

ng-template (serach and replace &lt;template)

@@ -208,17 +208,7 @@ Here is `*ngIf` displaying the hero's name if `hero` exists.


The asterisk is "syntactic sugar" for something a bit more complicated.
Internally, Angular desugars it in two stages.

This comment has been minimized.

Copy link
@vicb

vicb Aug 22, 2017

Contributor

sounds wrong.

I agree we should remove the "desugars it in two stages." it is wrong.



Then it translates the template _attribute_ into a `<ng-template>` _element_, wrapped around the host element, like this.
Internally, Angular translates the template _attribute_ into a `<ng-template>` _element_, wrapped around the host element, like this.

This comment has been minimized.

Copy link
@vicb

vicb Aug 22, 2017

Contributor

Angular translates the template _attribute_ this is wrong - this attribute is no more significant for Angular

This comment has been minimized.

Copy link
@ocombe

ocombe Aug 23, 2017

Author Contributor

the guide is talking about the *ngIf attribute actually, I'll change the phrasing

@vicb

This comment has been minimized.

Copy link
Contributor

vicb commented Aug 22, 2017

Agree with the code changes.

Not sure about the aio changes:

  • added some comments in the PR,
  • image resolution looks very different, is this expected ?
@juleskremer
Copy link
Contributor

juleskremer left a comment

LGTM with Vic changes; also, we need to check the sizing on the images.

@ocombe ocombe force-pushed the ocombe:disable-legacy-template branch from 72287eb to e5aa2e5 Aug 23, 2017

@mary-poppins

This comment has been minimized.

Copy link

mary-poppins commented Aug 23, 2017

@vicb

vicb approved these changes Aug 28, 2017

@IgorMinar
Copy link
Member

IgorMinar left a comment

Can we change this to a "feat" rather than "refactor" - since we are changing the behavior, we should not claim that this change is a refactor. refactorings should have no visible impact for external developers.

@ocombe ocombe force-pushed the ocombe:disable-legacy-template branch from e5aa2e5 to 63a8447 Aug 28, 2017

@ocombe ocombe changed the title refactor(compiler): set `enableLegacyTemplate` to false by default feat(compiler): set `enableLegacyTemplate` to false by default Aug 28, 2017

@ocombe

This comment has been minimized.

Copy link
Contributor Author

ocombe commented Aug 28, 2017

@IgorMinar done

@jasonaden

This comment has been minimized.

Copy link
Contributor

jasonaden commented Aug 29, 2017

@ocombe Please have this pass Travis, then we can merge.

feat(compiler): set `enableLegacyTemplate` to false by default
BREAKING CHANGE: the compiler option `enableLegacyTemplate` is now disabled by default as the `<template>` element has been deprecated since v4. Use `<ng-template>` instead. The option `enableLegacyTemplate` and the `<template>` element will both be removed in Angular v6.

@ocombe ocombe force-pushed the ocombe:disable-legacy-template branch from 63a8447 to daca25f Aug 29, 2017

@mary-poppins

This comment has been minimized.

Copy link

mary-poppins commented Aug 29, 2017

@jasonaden jasonaden closed this in 56238fe Sep 1, 2017

@ocombe ocombe deleted the ocombe:disable-legacy-template branch Sep 4, 2017

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.