PolymerElements / iron-a11y-keys-behavior

A behavior that enables keybindings for greater a11y
24 stars 41 forks source link

Polymer 3.2: TypeError cannot convert null to object #93

Open pdesjardins90 opened 5 years ago

pdesjardins90 commented 5 years ago

Description

When I try to upgrade from Polymer 3.1 to Polymer 3.2, When creating a paper-dialog (which depends on iron-a11y-keys-behavior), An error is thrown

Expected outcome

No error

Actual outcome

Uncaught TypeError: Cannot convert undefined or null to object (node_modules/@polymer/iron-a11y-keys-behavior/iron-a11y-keys-behavior.js:430) at HTMLElement._attachDom (node_modules/@polymer/polymer/lib/mixins/element-mixin.js:739:24) at HTMLElement._readyClients (node_modules/@polymer/polymer/lib/mixins/element-mixin.js:703:26) at HTMLElement._flushClients (node_modules/@polymer/polymer/lib/mixins/property-effects.js:1734:14) at HTMLElement._propertiesChanged (node_modules/@polymer/polymer/lib/mixins/property-effects.js:1883:12) at HTMLElement._flushProperties (node_modules/@polymer/polymer/lib/mixins/properties-changed.js:390:14) at HTMLElement._flushProperties (node_modules/@polymer/polymer/lib/mixins/property-effects.js:1716:13) at HTMLElement.ready (node_modules/@polymer/polymer/lib/mixins/property-effects.js:1835:12) at HTMLElement.ready (node_modules/@polymer/polymer/lib/mixins/element-mixin.js:687:13) at HTMLElement.ready (node_modules/@polymer/polymer/lib/mixins/dir-mixin.js:158:13)

Steps to reproduce

Browsers Affected

super-kamil commented 5 years ago

+1

super-kamil commented 5 years ago

Wrap paper-dialog by an own web component and use this as a workaround:

constructor() {
    super();
    this._keyBindings = this._keyBindings || {};
}
pdesjardins90 commented 5 years ago

I'll try that!

balloob commented 5 years ago

I am seeing the same thing. this._keyBindings should have been initialized in the registered callback, but it looks like that doesn't run. However, I don't get this error with all elements that use this. Have not been able to track it down yet.

dfreedm commented 5 years ago

Can you provide an example where this fails? I'm not seeing this behavior by running the tests of <paper-dialog>

balloob commented 5 years ago

I've been trying to recreate a standalone repro and have failed 😞

However, I can give you a repro in our demo that you can check out locally too.

The repro is deployed here: https://repro-a11y--home-assistant-demo.netlify.com

To repro:

Notes:

If you want to check out the repro locally:

dfreedm commented 5 years ago

This could be https://github.com/Polymer/polymer/commit/50ad018c

dfreedm commented 5 years ago

Hmm, I see you are extending paper-dropdown-menu here: https://github.com/home-assistant/home-assistant-polymer/blob/repro-a11y/src/components/ha-paper-dropdown-menu.ts

This is probably breaking the registration of paper-dropdown-menu itself somehow.

dfreedm commented 5 years ago

Ok, it looks like in 3.2, we tried to optimize behavior registration to behave more like Polymer 1.x, and mix behaviors onto the element prototype, rather than generating a mixin for every behavior.

However, this breaks your particular use case, because the first element to boot up will have the registered call run on it, which in your case the this subclass of paper-dropdown-menu. This means that the paper-dropdown-menu in the "History" section is missing what was set in registered.

We need to run registered on each subclass of the Polymer({}) generated class, which is more like the 3.1 behavior.