Avoid this.$refs needs context

Question

Avoid this.$refs needs context

kartsims opened this issue 8 years ago · 14 comments

While reading the paragraph about avoiding this.$refs I thought it wasn't clear enough that you mean "avoid it to access subcomponents".

this.$refs can also be used to access any DOM element's native API and in this case it is pretty useful and can't be done otherwise.

Would you prefer a PR ?

Answer 1 · 2017-02-08T17:24:28.000Z

Hello @kartsims ! Thank you for this issue.

OK, help me out.

What I meant is that instead using this.$refs on subcomponents, the subcomponent should provide a good API (props + events). That's why I said "avoid" instead of "don't".

I'm not against the use of refs to access DOM elements.

How could we write this in a more specific way?

Answer 2 · 2017-02-09T11:42:35.000Z

After trying to change the paragraph I came to the conclusion that it just doesn't belong here. Not like this anyway

Using this.$refs is a hacky way to overcome a component's poor API.

It is still interesting to mention it but maybe in a more general way, at the end of the style guide, in a paragraph like "Improving existing components" or "When should you change the component itself"... Not fan of either title but you get the idea.

I believe it would be interesting to mention specific cases. Examples :

1 - Accessing a subcomponent through this.$refs is a good example of a poor component's API. They should never be accessed via this.$refs.xxx as it violates the main rule of component based frameworks : props down, events up. This shortcut is a hacky way to highlight caveats in a component's API.

2 - Using a .sync props modifiers on a component is another example of disregarding the "props down, events up" rule. Your component should expose an event when the specific prop's value is updated or offer another way of doing so.

I am sure there are plenty of other examples you could think of. The times when you hesitated and thought "hmm maybe this is not standard, there must be a better way"

Answer 3 · 2017-02-10T09:40:10.000Z

What do you think about changing it to:

Avoid `this.$refs`

Vue.js supports components to have access to other components and elements context via ref attribute. That attribute will provide an accessible way through this.$refs to other components/elements context. The need to access other componets context via this.$refs is a hack. In most cases this.$refs just means a component lacks a good API.

Why?

A vue component, like any component, must work in isolation. If a component does not support all the access needed, it was bad designed/implemented.

How?

Create a good component API;
Check all the props to see if something is missing. If it is, create an issue or enhance the component yourself;
Check all the events. In most cases developers forget that Child-Parent communication (events) is needed, that's why they only remember the Parent-Child communication (using props);
Props down, events up! Upgrade your component when requested with a good API and isolation as goals.

<range :max="max"
  :min="min"
  @current-value="currentValue"
  :step="1"></range>


<!-- avoid -->
<template>
  <range :max="max"
    :min="min"
    ref="range"
    :step="1"></range>
</template>

<script>
  export default {
    // ...
    methods: {
      getRangeCurrentValue() {
      	return this.$refs.range.currentValue;
      },
    },
    // ...
  };
</script>

Answer 4 · 2017-02-10T10:11:20.000Z

I agree this.$refs is not always a bad practice. See this API for example: http://quasar-framework.org/components/modal.html

I think it's pretty clean to access modal open and close method. How would you do it otherwise?
Using a bool prop maybe? I think it wouldn't be as nice.

Answer 5 · 2017-02-10T10:53:14.000Z

Use $refs wisely, as quick access to dom objects in complex components as Modals. Yep.

Answer 6 · 2017-02-10T10:53:21.000Z

I never said its a bad practice @Elfayer . I said its better to avoid it. :)

The example you just shown its a good example of when use this.$refs. It makes sense and it looks super clean.

Check my last comment to see a bad use of this.$refs. Why is it bad? Because that value could be emitted. It makes sense and the component would be isolated.

What do you think @Elfayer ?

Answer 7 · 2017-02-10T11:08:58.000Z

@pablohpsilva I think this.$refs should be mentioned as a useful tool, but people should use it only when required. It could mean that the child component API is wrong.
Maybe just add a reminder: Use this.$refs as last resort, most of the time properties and events will be sufficient to a strong API.

We are talking about using this.$refs on components, right?
On basic HTML elements it's just fine to allow DOM manipulation. Isn't it? (still better than $ or document.getElement[...]/document.queryElement(), those should never appear in a VueJS code, from my point of view)
Although with data-binding DOM manipulation should not happen often.

Edit: Or maybe just mention to use it when you know why you use it, not just as a shortcut.

Also maybe add this image?
http://i.stack.imgur.com/G1YWd.png
It comes from The Progressive Framework slides of Evan You.

Answer 8 · 2017-02-10T11:26:54.000Z

Yeap! That's exactly it @Elfayer .

Guys, help me out writing this style guide:

Use `this.$refs` as last resort

Vue.js supports components to have access to other components and elements context via ref attribute. That attribute will provide an accessible way through this.$refs to other components/elements context. The need to access other componets context via this.$refs is a hack and in most cases this.$refs just means a component lacks a good API.

Why?

A vue component, like any component, must work in isolation. If a component does not support all the access needed, it was bad designed/implemented.
this.$refs is a useful tool, but you should know when to use it. See the How? below.

How?

Create a good component API;
Check all the props to see if something is missing. If it is, create an issue or enhance the component yourself;
Check all the events. In most cases developers forget that Child-Parent communication (events) is needed, that's why they only remember the Parent-Child communication (using props);
Props down, events up! Upgrade your component when requested with a good API and isolation as goals;
Using this.$refs on components should be used when props and event options are exhausted and having it makes sense (see the example below);
Using this.$refs to access DOM elements (instead of doing jQuery, document.getElement*, document.queryElement) is just fine, when the element can't be manipulated with data bindings.

<range :max="max"
  :min="min"
  @current-value="currentValue"
  :step="1"></range>

<!--good example of when to use this.$refs -->
<q-modal ref="basicModal">
  <h4>Basic Modal</h4>
  <button class="primary" @click="$refs.basicModal.close()">Close</button>
</q-modal>


<!-- avoid accessing something that could be emitted -->
<template>
  <range :max="max"
    :min="min"
    ref="range"
    :step="1"></range>
</template>

<script>
  export default {
    // ...
    methods: {
      getRangeCurrentValue() {
      	return this.$refs.range.currentValue;
      },
    },
    // ...
  };
</script>

Answer 9 · 2017-02-10T13:43:31.000Z

@pablohpsilva I added as much details as I could, keep what you like ;)
I made italic everything I changed.

Use `this.$refs` with caution

Vue.js supports components to have access to other components and basic HTML elements context via ref attribute. That attribute will provide an accessible way through this.$refs to a component or DOM element context. In most cases, the need to access other components context via this.$refs could be avoided. This is why you should be careful when using it to avoid wrong component APIs.

Why?

A vue component, like any component, must work in isolation. If a component does not support all the access needed, it was badly designed/implemented.
Properties and events should be sufficient to most of your components.

How?

Create a good component API;
Always focus on the component purpose out of the box;
Never write specific code. If you need to write specific code inside a generic component, it means its API isn't generic enough or maybe you need a new component to manage other cases;
Check all the props to see if something is missing. If it is, create an issue or enhance the component yourself;
Check all the events. In most cases developers forget that Child-Parent communication (events) is needed, that's why they only remember the Parent-Child communication (using props);
Props down, events up! Upgrade your component when requested with a good API and isolation as goals;
Using this.$refs on components should be used when props and events are exhausted and having it makes sense (see the example below);
Using this.$refs to access DOM elements (instead of doing jQuery, document.getElement*, document.queryElement) is just fine, when the element can't be manipulated with data bindings or for a directive.

<!-- good, no need for ref -->
<range :max="max"
  :min="min"
  @current-value="currentValue"
  :step="1"></range>

<!-- good example of when to use this.$refs -->
<modal ref="basicModal">
  <h4>Basic Modal</h4>
  <button class="primary" @click="$refs.basicModal.close()">Close</button>
</modal>
<button @click="$refs.basicModal.open()">Open modal</button>

<!-- Modal component -->
<template>
  <div v-show="active">
    <!-- ... -->
  </div>
</template>

<script>
  export default {
    // ...
    data() {
        return {
            active: false,
        };
    },
    methods: {
      open() {
      	this.active = true;
      },
      hide() {
      	this.active = false;
      },
    },
    // ...
  };
</script>

<!-- avoid accessing something that could be emitted -->
<template>
  <range :max="max"
    :min="min"
    ref="range"
    :step="1"></range>
</template>

<script>
  export default {
    // ...
    methods: {
      getRangeCurrentValue() {
      	return this.$refs.range.currentValue;
      },
    },
    // ...
  };
</script>

Answer 10 · 2017-02-10T13:46:31.000Z

I like this version, maybe we could continue the discussion (if need be) in a PR ?

Answer 11 · 2017-02-10T15:03:35.000Z

I liked it too and agree with @kartsims . @Elfayer could you please make that change on a PR for us?
:D

Answer 12 · 2017-02-10T16:21:00.000Z

@pablohpsilva I'll try, but it'll be my first commit ever on GitHub, so I hope I'm not going to break anything. =/

Answer 13 · 2017-02-10T16:27:36.000Z

Please tell me if I made any mistake in the process.

Answer 14 · 2017-02-13T14:45:51.000Z

Nope @Elfayer :D
PR accepted.

Avoid this.$refs

Why?

How?

Use this.$refs as last resort

Why?

How?

Use this.$refs with caution

Why?

How?

Avoid `this.$refs`

Use `this.$refs` as last resort

Use `this.$refs` with caution