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

fix(vdom): properly warn for step attr on input #3196

Merged
merged 1 commit into from
Dec 29, 2021

Conversation

rwaskiewicz
Copy link
Member

@rwaskiewicz rwaskiewicz commented Dec 29, 2021

Pull request checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)
  • Build (npm run build) was run locally and any changes were pushed
  • Unit tests (npm test) were run locally and passed
  • E2E Tests (npm run test.karma.prod) were run locally and passed
  • Prettier (npm run prettier) was run locally and passed

Pull request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

What is the current behavior?

Stencil performs dev-time checking of the order of input element attributes. The check for the step attribute is incorrect, as it points to min

GitHub Issue Number: #2406

What is the new behavior?

fix indexOf call for the step attribute to look for the correct
attribute name.

hoist the check for value attribute to exit early if the attribute
does not exist, avoiding unneeded lookups of other attributes

Does this introduce a breaking change?

  • Yes
  • No

Testing

First, I built this branch, npm ci && npm run clean && npm run build && npm pack.

I spun up a new Stencil component library (npm init stencil -> component), ran npm i for the component, then installed the tarball generated from the previous call to npm pack

Replace the render fn in src/my-component.tsx:

  render() {
    // note 'step' is after 'value'
    return <input value={"Hello"} step={1}></input>;
  }

Run npm start in a terminal window whose cwd is the root of the Stencil component library that was spun up. Open dev tools in your browser of choice to see the err msg.
Screen Shot 2021-12-29 at 9 41 11 AM

Similarly, removing 'step'/reordering the attributes removes the error message:

  render() {
    // note 'value' is after 'step'
    return <input step={1} value={"Hello"}></input>;
  }

@rwaskiewicz rwaskiewicz requested a review from a team December 29, 2021 14:41
fix `indexOf` call for the step attribute to look for the correct
attribute name.

hoist the check for `value` attribute to exit early if the attribute
does not exist, avoiding unneeded lookups of other attributes

STENCIL-79: Fix typo on <input> attribute validations
@rwaskiewicz rwaskiewicz force-pushed the stencil-79/fix-input-attr-validation branch from 5a34b12 to 0e717f1 Compare December 29, 2021 14:44
@rwaskiewicz rwaskiewicz merged commit 7ffc02e into main Dec 29, 2021
@rwaskiewicz rwaskiewicz deleted the stencil-79/fix-input-attr-validation branch December 29, 2021 15:00
rwaskiewicz added a commit that referenced this pull request Dec 30, 2021
fix `indexOf` call for the step attribute to look for the correct
attribute name.

hoist the check for `value` attribute to exit early if the attribute
does not exist, avoiding unneeded lookups of other attributes

STENCIL-79: Fix typo on <input> attribute validations
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants