feat(biome_js_analyze): noVueDataObjectDeclaration #6574
Conversation
|
crates/biome_js_analyze/src/lint/nursery/no_vue_data_object_declaration.rs
Outdated
Show resolved
Hide resolved
| fn run(ctx: &RuleContext<Self>) -> Self::Signals { | ||
| let member = ctx.query(); | ||
|
|
||
| if let Ok(member_name) = member.name() { | ||
| if let Some(name) = member_name.name() { | ||
| if name.trim() == "data" { | ||
| if let Ok(value) = member.value() { | ||
| match value { | ||
| AnyJsExpression::JsIdentifierExpression(_) | ||
| | AnyJsExpression::JsArrowFunctionExpression(_) | ||
| | AnyJsExpression::JsFunctionExpression(_) => return None, | ||
| _ => return Some(()), | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Needs to be filtered so that it only runs in .vue files.
crates/biome_js_analyze/src/lint/nursery/no_vue_data_object_declaration.rs
Outdated
Show resolved
Hide resolved
| if let Ok(member_name) = member.name() { | ||
| if let Some(name) = member_name.name() { | ||
| if name.trim() == "data" { | ||
| if let Ok(value) = member.value() { | ||
| match value { | ||
| AnyJsExpression::JsIdentifierExpression(_) | ||
| | AnyJsExpression::JsArrowFunctionExpression(_) | ||
| | AnyJsExpression::JsFunctionExpression(_) => return None, | ||
| _ => return Some(()), | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
This needs to make sure that the data member is at the specific level that we are expecting. This would be a false positive:
{
methods: {
foo() {
const bar = {
data: {}
}
}
}
}There was a problem hiding this comment.
d1c004b - I'm hoping this covers all the cases we need. Let me know if i'm still missing any here. I added this example + the test cases from both eslint rules in the snapshot tests
…t rule test source
…nly check for data member in vue options objects, write documentation, update action
| pub NoVueDataObjectDeclaration { | ||
| version: "next", | ||
| name: "noVueDataObjectDeclaration", | ||
| language: "vue", | ||
| recommended: true, | ||
| fix_kind: FixKind::Safe, | ||
| domains: &[RuleDomain::Vue], | ||
| } |
There was a problem hiding this comment.
Whoops, I missed this on my first pass -- This is missing the rule source that points to the original rule.
| // createApp(...), defineComponent(...) | ||
| AnyJsExpression::JsIdentifierExpression(_) => { | ||
| let options_argument = call.arguments().ok()?.args().elements().next()?.into_node(); |
There was a problem hiding this comment.
I feel like the strings need to be checked here to make sure we don't hit false positives. As in, we need to make sure this is a call to createApp or defineComponent (PS -- you'll want to compare the TokenText, not a String).
That being said, I'm not sure if its worth invoking the semantic model here to make sure this is imported from vue. Since this rule is confined to the vue domain, I would think anyone calling these functions would be importing them from vue. So the simple string comparison will suffice.
| if let Ok(object_member_name) = property_object_member.name() { | ||
| if let Some(key_name) = object_member_name.name() { | ||
| if key_name.text().trim() == "data" { | ||
| if let Ok(value) = property_object_member.value() { | ||
| match value { |
There was a problem hiding this comment.
nit: You might want to try chaining .and_then() instead to reduce this indentation
| i Safe fix: Convert the data object to a function returning the data object | ||
|
|
||
| 9 9 │ // app.component with object `data` | ||
| 10 10 │ app.component('some-comp', { | ||
| 11 │ - ··data:·{ | ||
| 11 │ + ··data:·function(){return{ | ||
| 12 12 │ foo: 'bar' | ||
| 13 │ - ··} | ||
| 13 │ + ··}} | ||
| 14 14 │ }); | ||
| 15 15 │ |
There was a problem hiding this comment.
Generally, we try to make our fixes line up with what the formatter would output so the user doesn't have to run biome twice.
|
@amousa11 Would you mind if I took over this PR? We have some more generic vue tooling coming in another PR that this would benefit from. |
|
@dyc3 No problem, I just pushed up my WIP based on your feedback. Can you link me to that PR for my own learning? |
Summary
Port
vue/no-deprecated-data-object-declarationand/vue/no-shared-component-datafromeslint-plugin-vue-#6300closes #6300
Test Plan
vue/no-deprecated-data-object-declaration/vue/no-shared-component-datafromeslint-plugin-vue#6300