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

Warn destructing attrs & slots #1964

Closed
jd-solanki opened this issue Sep 9, 2022 · 12 comments
Closed

Warn destructing attrs & slots #1964

jd-solanki opened this issue Sep 9, 2022 · 12 comments
Labels
needs repro Need a repository that can reproduce the problem, or a link to DEMO.

Comments

@jd-solanki
Copy link

jd-solanki commented Sep 9, 2022

Please describe what the rule should do:

What category should the rule belong to?

[ ] Enforces code style (layout)
[x] Warns about a potential error (problem)
[ ] Suggests an alternate way of doing something (suggestion)
[ ] Other (please specify:)

Provide 2-3 code examples that this rule should warn about:

setup(props, { attrs, slots }) {
  const { slotFoo, slotBar } = slots
  const { id, hidden } = attrs
}

Additional context
According to vue docs, destructured attrs & slots aren't reactive. Hence, we should not destructure them in setup function.

attrs and slots are stateful objects that are always updated when the component itself is updated. This means you should avoid destructuring them and always reference properties as attrs.x or slots.x. Also note that, unlike props, the properties of attrs and slots are not reactive.

EDIT: Sample code updated, Thanks @FloEdelmann

@FloEdelmann

This comment was marked as resolved.

@jd-solanki

This comment was marked as resolved.

@jd-solanki jd-solanki changed the title vue/no-setup-slot-destructure & vue/no-setup-attrs-destructure Disable destructing attrs & slots Sep 9, 2022
@jd-solanki jd-solanki changed the title Disable destructing attrs & slots Warn destructing attrs & slots Sep 9, 2022
@FloEdelmann
Copy link
Member

We have a similar rule: https://eslint.vuejs.org/rules/no-setup-props-destructure.html

Maybe it could be modified to also check attrs and slots.

@FloEdelmann
Copy link
Member

Related to #1949.

@ota-meshi
Copy link
Member

Hmm...
I don't think attrs and slots are reactive, destructured or not.

So they should be used more carefully. I find it difficult to check well.

For example, use onBeforeUpdate to handle:

setup(props, { attrs, slots }) {
  let { slotFoo, slotBar } = slots
  let { id, hidden } = attrs
  // ...
  onBeforeUpdate(() => {
    ;({ slotFoo, slotBar } = slots)
    ;({ id, hidden } = attrs)
    // ...
  })
}

Using destructuring, but I don't think the above is wrong.

@jd-solanki
Copy link
Author

I don't think attrs and slots are reactive

They are stateful as mentioned in docs

@ota-meshi
Copy link
Member

Yes. In other words, the example code you provided is neither right nor wrong.
Even with destructuring, it will probably behave correctly depending on how the value of is handled.

setup(props, { attrs, slots }) {
   const { slotFoo, slotBar } = slots
   const { id, hidden } = attrs
}

What would your proposed rule check for?

@jd-solanki
Copy link
Author

What would your proposed rule check for?

It will assure that we don't destructure the attrs & slots.

In my lib anu-vue I previously had this destructuring which was causing reactivity lost.

E.g. If I pass type attribute as ref and change it via button click then its type value doesn't get updated from text to password.

Thanks.

@ota-meshi
Copy link
Member

Did you fix it? I think the attrs.id of the main branch is still lost in reactivity.

https://github.com/jd-solanki/anu/blob/6796c843812cb9ec3cd1e0cd337866df40967439/packages/anu-vue/src/components/base-input/ABaseInput.tsx#L24

@jd-solanki
Copy link
Author

I think the attrs.id of the main branch is still lost in reactivity

Let me check

@jd-solanki
Copy link
Author

jd-solanki commented Sep 12, 2022

Below code in my playground is working fine on main branch:

<script lang="ts" setup>
import { ref } from "vue";

const inputType = ref("password");
const toggleType = () => {
  console.log("clicked");
  inputType.value = inputType.value === "text" ? "password" : "text";
};
</script>

<template>
    <AInput :type="inputType" modelValue="text"></AInput>
    <ABtn @click="toggleType">Toggle type</ABtn>
</template>

It is working fine:
chrome-capture-2022-8-12

Now even docs is confusing me 🤣

It states attrs & slots "stateful objects". Moreover, there's also "properties of attrs and slots are not reactive".

IG someone better than me needs to comment on this or I should try this in SFC playground.

@ota-meshi
Copy link
Member

Can you provide a SFC playground link to try out your components?

@FloEdelmann FloEdelmann added the needs repro Need a repository that can reproduce the problem, or a link to DEMO. label Feb 27, 2023
@FloEdelmann FloEdelmann closed this as not planned Won't fix, can't repro, duplicate, stale May 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs repro Need a repository that can reproduce the problem, or a link to DEMO.
Projects
None yet
Development

No branches or pull requests

3 participants