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(filter): remove unsupported Vue.filter #81

Conversation

Robo-Rin
Copy link

@Robo-Rin Robo-Rin commented Jul 11, 2023

Description

Vue 3 upgrade gives errors due to the deprecated Vue.filter. This PR fixes this issue by removing the filter altogether. As mentioned by Ronald further down we can use the already exported masker directly instead.

For example:

<template>
  <div> {{ masker('1234', '###-#').masked }} </div>
</template>

<script>
import { masker } from 'vue-input-facade'

export default {
  setup() {
    return { masker }
  }
}

</script>

Note: Not marking this as a breaking change as the existing beta branch was already broken for Vue 3.

Checklist

  • Tests
  • Documentation
  • Used commitizen and followed Conventional Commits
  • Commit footer references issue num. If applicable.

@Robo-Rin Robo-Rin force-pushed the fix/vue3-replace-filter-with-utility-function branch from 32fd587 to b92dc9d Compare July 11, 2023 23:41
describe('Plugin tests', () => {
describe('useFacade', () => {
const ComponentMock = {
template: '<div id="maskedText">{{ useFacade(input, facadeConfig) }}</div>',
Copy link
Contributor

@Umi8Umi Umi8Umi Jul 12, 2023

Choose a reason for hiding this comment

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

maybe good to edit the readme section for filters to show this new syntax as well

there is also the filter.md which is shown in the styleguide for the intput facade which would be nice to update with this vue3 compatible syntax

Copy link
Author

Choose a reason for hiding this comment

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

Good call to edit README 👍

src/plugin.js Outdated
*
* @param {String} value the value to apply the filter to
* @param {*} config the masking config
* @returns {string} the masked value as returned by the masker function
*/
function filter(value, config) {
function useFacade(value, config) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nice!!!

src/plugin.js Outdated
*
* @param {String} value the value to apply the filter to
* @param {*} config the masking config
* @returns {string} the masked value as returned by the masker function
*/
function filter(value, config) {
function useFacade(value, config) {
Copy link
Owner

Choose a reason for hiding this comment

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

Since the filters are no longer supported then we should just remove this function and anyone who needs it can just use the masker function which is already been exported. This was just a wrapper around that function for syntax sugar in vue2.

Copy link
Author

@Robo-Rin Robo-Rin Jul 12, 2023

Choose a reason for hiding this comment

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

  • fix

Vue 3 doesn't support Vue.filter anymore but we can use the masker directly instead.

closes RonaldJerez#44
@Robo-Rin Robo-Rin force-pushed the fix/vue3-replace-filter-with-utility-function branch from b92dc9d to 344ff9d Compare July 12, 2023 15:26
@Robo-Rin Robo-Rin changed the title fix(filter): replace unsupported Vue.filter fix(filter): remove unsupported Vue.filter Jul 12, 2023
@RonaldJerez RonaldJerez merged commit 2fe4afd into RonaldJerez:beta Jul 12, 2023
2 checks passed
@github-actions
Copy link

🎉 This PR is included in version 3.0.0-beta.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

Robo-Rin added a commit to Robo-Rin/vue-input-facade that referenced this pull request Jan 4, 2024
Vue 3 doesn't support Vue.filter anymore but we can use the masker directly instead.

closes RonaldJerez#44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants