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

Simple prop declaration replaces "" with nothing when auto-fixing #993

Closed
carestad opened this issue Nov 25, 2019 · 5 comments · Fixed by #1009
Closed

Simple prop declaration replaces "" with nothing when auto-fixing #993

carestad opened this issue Nov 25, 2019 · 5 comments · Fixed by #1009
Labels

Comments

@carestad
Copy link

Tell us about your environment

  • ESLint version: v6.7.1
  • eslint-plugin-vue version: v6.0.1
  • Node version: v12.13.0

Please show your full configuration:

{
    "extends": [
        "eslint:recommended",
        "plugin:vue/recommended"
    ],
    "plugins": [
        "vue"
    ]
}

What did you do?

<template>
  <div>
    <h1>Hello</h1>
  </div>
</template>

<script>
export default {
  name: 'Test',
  props: {
    foo: "",
  },
};
</script>

Running eslint test.vue gives me the following:

image

And then after running eslint --fix test.vue

image

What did you expect to happen?

Not sure what I expected, but did not expect it to just replace "" with nothing, leaving the JS invalid.

What actually happened?

{
  props: {
    foo: "",
  }
}

was turned into

{
  props: {
    foo: ,
  }
}
@LinusBorg
Copy link
Member

LinusBorg commented Nov 25, 2019

I'd say this rule isn't really be --fix-able , as we can at best take an educated guess that you meant it to be a string and should be foo: String instead of foo: "".

@carestad
Copy link
Author

carestad commented Nov 25, 2019

I'd say this rule isn't really be --fix-able , as we can at best take an educated guess that you meant it to be a string and should be foo: String instead of foo: "".

Would that be possible? Or maybe rather assume that foo: "" could be rewritten to foo: null (allow any type) at least. Or just skip these declarations altogether.

@LinusBorg
Copy link
Member

LinusBorg commented Nov 25, 2019

Since the developer intent can't be properly inferred, silently assuming null when he developer may expect the prop validation to fail when something besides a string is passed would be dangerous.

The developer appearantly wanted to define the prop (likely to be a String), but failed to do so in the correct way/syntax, so it should simply fail and not be corrected, leaving that to the developer.

@carestad
Copy link
Author

Since the developer intent can't be properly inferred, silently assuming null when he developer may expect the prop validation to fail when something besides a string is passed would be dangerous.

The developer appearantly wanted to define the prop (likely to be a String), but failed to do so in the correct way/syntax, so it should simply fail and not be corrected, leaving that to the developer.

Yeah, I agree on that.

But why is "" even removed in the first place and not just left as-is? It would at least still work afterwords.

@ota-meshi
Copy link
Member

Thank you for this issue.

I think it is a bug that the code is deleted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants