material-components / material-components-web

Modular and customizable Material Design UI components for the web
https://material.io/develop/web
MIT License
17.13k stars 2.15k forks source link

mdc-textfield assumes NativeInput.validity is an object when it may be undefined #3983

Open JSMike opened 5 years ago

JSMike commented 5 years ago

Bugs

** Unable to use codepen for server-side rendering example.

What MDC Web Version are you using?

0.40.1

What browser(s) is this bug affecting?

Server side rendering with Angular Universal

What OS are you using?

Windows

What are the steps to reproduce the bug?

Create Angular Universal Server and load page with a text-field.

What is the expected behavior?

No errors

What is the actual behavior?

ERROR TypeError: Cannot read property 'valid' of undefined

Any other information you believe would be useful?

https://github.com/material-components/material-components-web/blob/6b3cfe5f3ee1158593f63ff77b7537b36e87dcfb/packages/mdc-textfield/foundation.js#L473

https://github.com/material-components/material-components-web/blob/6b3cfe5f3ee1158593f63ff77b7537b36e87dcfb/packages/mdc-textfield/foundation.js#L410

https://github.com/material-components/material-components-web/blob/6b3cfe5f3ee1158593f63ff77b7537b36e87dcfb/packages/mdc-textfield/foundation.js#L402

The issue is that Domino, which is used by Angular Universal for Server Side Rendering, doesn't have a ValidityState object on it's HTMLInputElement implementation and the mdc-textfield foundation assumes that if NativeInput returns it will have a validity property that is an object. Foundation functions call validity.valid and validity.badInput without checking if the validity property exists and is an object.

Suggested Fix

This can be resolved by updating the getNativeInput_ function to spread the returned value into the defined default, instead of returning one or the other:

function getNativeInput_() {
  return /** @type {!NativeInputType} */ {
    value: '',
    disabled: false,
    validity: {
      badInput: false,
      valid: true,
    },
    ...this.adapter_.getNativeInput()
  };
}
acdvorak commented 5 years ago

Thanks for filing this issue!

I assume your suggested fix refers to this bit of code:

https://github.com/material-components/material-components-web/blob/6b3cfe5f3ee1158593f63ff77b7537b36e87dcfb/packages/mdc-textfield/foundation.js#L472-L482

We probably can't use the spread operator that way in MDC, because it will copy all enumerable properties from the live DOM node.

Ideally, the server side rendering framework should provide a validity property, but if they are unwilling to add it, you could copy (or subclass) MDCTextField and override getNativeInput() in getInputAdapterMethods_():

https://github.com/material-components/material-components-web/blob/6b3cfe5f3ee1158593f63ff77b7537b36e87dcfb/packages/mdc-textfield/index.js#L477-L483

E.g.:

getNativeInput: () => {
  if (!this.input_.validity) {
    this.input_.validity = {
      badInput: false,
      valid: true,
    };
  }
  return this.input_;
}

I know it's not ideal, but I'm not sure what else we can do on the MDC side. Ultimately, we have to rely on the native APIs available in major browsers, and unfortunately server-side rendering is not something we officially support right now.

That said, we'll do some investigation and see if there's a way we can support your use case 😀