-
Notifications
You must be signed in to change notification settings - Fork 87
feat: warn if using legacy Template API #2101
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
Changes from all commits
90b1355
4353536
2375f0b
cd98870
0bc3a36
4861609
871d3d8
793b80d
2407498
6ac141e
3d48f1f
b492b47
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| /** | ||
| * @license | ||
| * Copyright (c) 2021 Vaadin Ltd. | ||
| * This program is available under Apache License Version 2.0, available at https://vaadin.com/license/ | ||
| */ | ||
|
|
||
| /** | ||
| * Passes the component to the template renderer callback if the template renderer is imported. | ||
| * Otherwise, if there is a template, it warns that the template renderer needs to be imported. | ||
| * | ||
| * @param {HTMLElement} component | ||
| */ | ||
| export function processTemplates(component) { | ||
| if (window.Vaadin && window.Vaadin.templateRendererCallback) { | ||
| window.Vaadin.templateRendererCallback(component); | ||
| return; | ||
| } | ||
|
|
||
| if (component.querySelector('template')) { | ||
| console.warn( | ||
| `WARNING: <template> inside <${component.localName}> is no longer supported. Import @vaadin/vaadin-template-renderer to enable compatibility (see https://vaad.in/template-renderer)` | ||
| ); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,71 @@ | ||
| import sinon from 'sinon'; | ||
| import { fixtureSync } from '@vaadin/testing-helpers'; | ||
| import { expect } from '@esm-bundle/chai'; | ||
| import { PolymerElement } from '@polymer/polymer/polymer-element.js'; | ||
|
|
||
| import { processTemplates } from '../templates.js'; | ||
|
|
||
| class ComponentElement extends PolymerElement { | ||
| static get is() { | ||
| return 'component-element'; | ||
| } | ||
|
|
||
| ready() { | ||
| super.ready(); | ||
|
|
||
| processTemplates(this); | ||
| } | ||
| } | ||
|
|
||
| customElements.define(ComponentElement.is, ComponentElement); | ||
|
|
||
| describe('process templates', () => { | ||
| beforeEach(() => { | ||
| sinon.stub(console, 'warn'); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| console.warn.restore(); | ||
| }); | ||
|
|
||
| describe('with template renderer', () => { | ||
| beforeEach(() => { | ||
| window.Vaadin = window.Vaadin || {}; | ||
| window.Vaadin.templateRendererCallback = sinon.spy(); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| window.Vaadin.templateRendererCallback = undefined; | ||
| }); | ||
|
|
||
| it('should invoke the template renderer callback', () => { | ||
| const component = fixtureSync(` | ||
| <component-element> | ||
| <template></template> | ||
| </component-element> | ||
| `); | ||
|
|
||
| expect(window.Vaadin.templateRendererCallback.calledOnce).to.be.true; | ||
| expect(window.Vaadin.templateRendererCallback.args[0][0]).to.equal(component); | ||
| }); | ||
|
|
||
| it('should not show the deprecation warning', () => { | ||
| expect(console.warn.called).to.be.false; | ||
| }); | ||
| }); | ||
|
|
||
| describe('without template renderer', () => { | ||
| it('should show the deprecation warning', () => { | ||
| fixtureSync(` | ||
| <component-element> | ||
| <template></template> | ||
| </component-element> | ||
| `); | ||
|
|
||
| expect(console.warn.calledOnce).to.be.true; | ||
| expect(console.warn.args[0][0]).to.equal( | ||
| `WARNING: <template> inside <component-element> is no longer supported. Import @vaadin/vaadin-template-renderer to enable compatibility (see https://vaad.in/template-renderer)` | ||
| ); | ||
| }); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -47,16 +47,6 @@ describe('renderer', () => { | |
| }; | ||
| }); | ||
|
|
||
| it('should remove template when added after renderer', () => { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is a no longer relevant test since we've made the template renderer responsible for throwing such exceptions. I missed this test while removing the Template API as far as it successfully passes by the reason of another exception ( |
||
| notification.renderer = () => {}; | ||
| const template = document.createElement('template'); | ||
| expect(() => { | ||
| notification.appendChild(template); | ||
| notification._observer.flush(); | ||
| }).to.throw(Error); | ||
| expect(notification._notificationTemplate).to.be.not.ok; | ||
| }); | ||
|
|
||
| it('should be possible to manually invoke renderer', () => { | ||
| notification.renderer = sinon.spy(); | ||
| notification.opened = true; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -85,15 +85,13 @@ class TimePickerElement extends ElementMixin(ControlStateMixin(ThemableMixin(Pol | |
| </style> | ||
| <vaadin-combo-box-light | ||
| allow-custom-value="" | ||
| item-label-path="value" | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| filtered-items="[[__dropdownItems]]" | ||
| disabled="[[disabled]]" | ||
| readonly="[[readonly]]" | ||
| auto-open-disabled="[[autoOpenDisabled]]" | ||
| dir="ltr" | ||
| theme$="[[theme]]" | ||
| > | ||
| <template> [[item.label]] </template> | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This template is not necessary here as by default the combo-box already renders |
||
| <vaadin-time-picker-text-field | ||
| class="input" | ||
| name="[[name]]" | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I forgot to import the template renderer here while removing the Template API as all the test cases in this file successfully pass regardless of a provided template (reason: tests don't actually use this template).
Missing has become noticeable since we are showing the warning: