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

refactor($compile): Disable warn if v-model is used with ternary text-like types for input. #6227

Conversation

nickmessing
Copy link
Member

What kind of change does this PR introduce? (check at least one)

  • Bugfix
  • Feature
  • Code style update
  • Refactor
  • Build-related changes
  • Other, please describe:

Does this PR introduce a breaking change? (check one)

  • Yes
  • No

If yes, please describe the impact and migration path for existing applications:

The PR fulfills these requirements:

If adding a new feature, the PR's description includes:

  • A convincing reason for adding this feature (to avoid wasting your time, it's best to open a suggestion issue first and wait for approval before working on it)

Other information:

#6158

Disable warn if v-model is used with ternary text-like types for input
@nickmessing nickmessing force-pushed the fix/unnecesarry-warn-v-model-ternary branch from 5bfb797 to a62d82a Compare July 26, 2017 11:30
@@ -4,6 +4,7 @@ import config from 'core/config'
import { addHandler, addProp, getBindingAttr } from 'compiler/helpers'
import { genComponentModel, genAssignmentCode } from 'compiler/directives/model'

const ternaryTextInputRE = /^[^?]{1,}\?\s*('|"|`)(?:text|number|password|search|email|tel|url)\1\s*:\s*('|"|`)(?:text|number|password|search|email|tel|url)\2\s*$/
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does a regexp like this introduce too much complexity that's not worth it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't use makeMap be enough? Don't we want to check if dynamic type is one of those?

Copy link
Member

@posva posva Aug 11, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, you actually did that an #6344
we need the opposite 😆

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@posva, #6344 is about compiling itself while this PR is just to remove Warning in this specific scenario.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant for the makeMap function 😆

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, yeah, we could use

/^[^?]{1,}\?\s*('|"|`)([a-z]*)\1\s*:\s*('|"|`)([a-z]*)\3\s*$/

and just checks [2] and [4] with makeMap. I'll add this changes to #6344 instead because isTextInputType is moved over there.

@nickmessing
Copy link
Member Author

Move to #6344

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.

2 participants