Refs are undefined in setup() outside of lifecycle method about composition-api HOT 20 CLOSED

@liximomo I don't agree. That negates the flexibility of the custom hook. In your example the ref cannot be specified at runtime - it is "prenamed".

from composition-api.

I appreciate you talking with me about this, but I don't think we're on the same page. Perhaps I'm not communicating this clearly.

I did not mean the actual context. I just meant that the refs are undefined and they are part of context.

Your latest example still is not allowing the developer to declare which ref should be used. It is hard-coded to use the 'elementKeypress' ref.

@yyx990803 What do you think about this composition pattern and issue? I believe that refs (and other context) should be usable at the first level of setup(), whereas currently they are 'undefined'. The two options I see would be to either keep the existing API of defining refs in the markup and utilizing it via context, OR to define refs via another new API, like React does with useRef().

from composition-api.

If this is the case, I think the RFC example code above should be altered because refs.foo in the onClick method would NOT be defined

The ref.foo will exist because if the onClick is called the component has already been mounted. Am I right?

from composition-api.

We should do this:

function useElementKeypress(key, refs, callback) {
  onMounted(() => {
    refs.elementKeypress.addEventListener("keydown", onKeydown);
  });
}

useElementKeypress("keydown", context.refs, () => {
  console.log("clicked!");
});

from composition-api.

Perhaps this is an issue to be dealt with in the API proposal. In React there is useRef- you define the ref in the component declaration and then bind that "variable" to the markup afterward. If Vue's new setup() function is supposed to be thought of running before the markup is generated, perhaps a new method needs to be added to the proposal for defining refs like const ref = reference() and then in the markup: <div :ref="ref"></div>.

from composition-api.

You could pass a function for the flexibility,

function useElementKeypress(key, getElement, callback) {
  onMounted(() => {
    getElement().addEventListener("keydown", onKeydown);
  })
}

useElementKeypress("keydown", () => context.refs.someElement, () => {
  console.log("clicked!");
});

from composition-api.

As I mentioned at the start of the issue, calling the custom hook inside of onMounted will not work because the hook has its own lifecycle methods. If I were to remove those, it does not work like the rest of the custom hooks - it would have restrictions that other hooks do not have.

The function is a better alternative, but it still looks strange.

I think that perhaps these are band-aid methods instead of solving the root issue.
Edit: One of the purposes of these hooks/effects is to be able to use them with each other to compose. If I cannot pass in context because of special requirements, I have lost the ability to compose.

from composition-api.

If I cannot pass in context because of special requirements, I have lost the ability to compose.

You can always pass context.

function useElementKeypress(key, ctx, callback) {
  onMounted(() => {
    ctx.refs.elementKeypress.addEventListener("keydown", onKeydown);
  });
}

useElementKeypress("keydown", context, () => {
  console.log("clicked!");
});

from composition-api.

If you really like useRef of React, we can create one wilt a little magic.

Notice how we pass ref in template

<template>
  <div class="hello">
    <h1>{{ message }}</h1>
    <h2>{{ messageUppercase }}</h2>
    <hr>

    <Counter/>
    <ExportsTest value="props!"/>

    <div :ref="elementKeypress" class="element-keypress-demo">Press "u" in here!</div>
  </div>
</template>

<script>
import Vue from "vue";
import { value, computed, onMounted } from "vue-function-api";
import Counter from "./Counter";
import ExportsTest from "./ExportsTest";
import useElementKeypress from "../hooks/useElementKeypress.js";

let refId = 0;
function useRef(ctx) {
  const name = `$ref${refId++}`;
  return Object.create(null, {
    current: {
      get() {
        return ctx.refs[name];
      },
    },
    toString: {
      value() {
        return name;
      },
    },
  });
}

function useElementKeypress(key, ref, callback) {
  function onKeydown(event) {
    const pressed = event.key;
    if (key === pressed) {
      callback();
    }
  }

  onMounted(() => {
    ref.current.addEventListener('keydown', onKeydown);
  });

  onBeforeDestroy(() => {
    ref.current.removeEventListener('keydown', onKeydown);
  });
}

const HelloWorld = Vue.extend({
  components: {
    Counter,
    ExportsTest
  },

  setup(props, context) {
    const message = value("Hello Vue in CodeSandbox!");
    const messageUppercase = computed(() => message.value.toUpperCase());

    const elementKeypress = useRef(context);
    useElementKeypress("u", elementKeypress, () => {
      console.log("clicked!");
    });

    return { message, messageUppercase, elementKeypress };
  }
});

export default HelloWorld;
</script>

from composition-api.

While this better (though I couldn't get it to work in the sandbox), I still think it is secondary to the refs actually working as expected when passed into the setup function.

I get that the function defines what should happen before the component is mounted, so the ref would not actually exist, but I think that is a design issue that needs to be brainstormed over.

Without something like the equivalent of useRef (like you created above), then the extraction of event binding is unable to happen. The component would be required to call a binding inside of onMounted and then a removal on onUnmounted/onBeforeDestroy. That is exactly the kind of problem the new RFC is trying to solve. Those things should be able to be wrapped up into their own "hook".

from composition-api.

The working example of useRef.

I get that the function defines what should happen before the component is mounted, so the ref would not actually exist, but I think that is a design issue that needs to be brainstormed over.

Should we call this a design issue:

<!DOCTYPE html>
<html lang="en">
  <head>
    <script>
      var appEl = document.getElementById('app');
      // oops! appEl is null.
    </script>
  </head>
  <body>
    <div id="app"></div>
  </body>
</html>

from composition-api.

That is a very valid point. I was not saying that the current design is bad. I was trying to say that the design I was originally trying to use was more "natural" and makes more sense, so others would probably run into the same issue- thus the issue is that people would get hung up.

Your example could also be done in userland, but do you think that this is more appropriate to put into the function API itself? I don't see it as just a common utility. It seems more like a standard.

There's something different between your example above (with getElementById) and what we are discussing. If I check the context inside of onMounted, I can get the ref. If I check it outside of it, it is undefined. The issue here is passing the reference because it is undefined. Perhaps the context outside of a lifecycle hook could be a reference (not undefined) that allows it to be passed elsewhere to use inside of a lifecycle hook. In my case, I am using the ref inside of onMounted which is inside of the hook. So in my mind, my implementation was not incorrect- it is that the ref did not pass by value because it was simply undefined.

I think my usage is more akin to this, which would work:

<!DOCTYPE html>
<html lang="en">
  <head>
    <script>
      var appEl = document.getElementById('app');
      // oops! appEl is null.

      // similar to onMounted
      $(function() {
        // works!
        var appEl = document.getElementById('app');
      });
    </script>
  </head>
  <body>
    <div id="app"></div>
    <script src=".../some.jquery.cdn"></script>
  </body>
</html>

Yes, I know jQuery's ready() method is not the same as onMounted(), but for the sake of this API, people will use it the same way. A reference should be exactly that- a reference to the virtual DOM node to be able to be manipulated. If context is going to pass it in, people will expect the reference to exist. They will probably understand that it cannot be directly executed upon outside of lifecycle methods. They will probably be confused like I was when they cannot pass by reference.

I hope that was clearer. Again, I appreciate you working through this with me. I'm not just trying to force the API to match my style. I really do want to help make this as robust and fluid as possible.

from composition-api.

I got your points. We all try to make thing better. But I think your original usage is more akin to this, which would not work:

<script>
  function useClick(el, cb) {
    $(function() {
      // not works!
      el.addEventListener('click', cb);
    });
  }

  var appEl = document.getElementById('app');
  useClick(appEl)
</script>
<div id="app"></div>

from composition-api.

Okay. I definitely see that. 😁 This IS because document.getElementById() is acting as a transaction though. Vue is not bound by this when determining the context. It could do something similar to your custom useRef function. It could create reactive objects for any refs it finds in the template (look-ahead) and pass those through context. Then when it gets to actually building the template it could "hydrate" the vDOM with the matching ref object.

I know that could be a change of the core, but I think it's at least a possible solution.

from composition-api.

One thing to add based on what I found in the RFC

attrs, slots and refs are in fact proxies to the corresponding values on the internal component instance. This ensures they always expose the latest values even after updates, so we can destructure them without worrying accessing a stale reference:

And this is the code example given:

const MyComponent = {
  setup(props, { refs }) {
    // a function that may get called at a later stage
    function onClick() {
      refs.foo // guaranteed to be the latest reference
    }
  }
}

from composition-api.

It could create reactive objects for any refs it finds in the template (look-ahead) and pass those through context. Then when it gets to actually building the template it could "hydrate" the vDOM with the matching ref object.

That's indeed possible technically, in theory. But it's definitely outside of the scope of the function-base API proposal itself, as it touches the template-compiler, build tools like webpack-loader etc.

As it currently stands, the problem you are facing exists with the current API just as well:

export default {
  beforeCreated() {
    console.log(this.$refs.something) / /=> undefined

    // setup runs at this moment, basically
  },
  created() {
    console.log(this.$refs.something) / /=> undefined
  },
  mounted() {
    console.log(this.$refs.something) / /=> HTMLElement
  }
}

Don't get me wrong, I like the idea, but what you propose would be an RFC of its own, as it's not so much related to the function-based API proposal as it is to the way Vue handles template refs.

from composition-api.

@LinusBorg If this is the case, I think the RFC example code above should be altered because refs.foo in the onClick method would NOT be defined. It would only be defined in onMounted inside of the scope of the containing component.

It could also be stated that Refs cannot be used in composition without a userland ref-creation OR by passing context and losing the ability to specify which ref (which I don't believe is true composition, but is in the examples above). The new way of utilizing refs are almost obsolete when using the new ability to compose extracted logic.

from composition-api.

@skyrpex Yes, but only if it is called inside of the component. I guess from the name of that method, we can assume that it is called from the template, but it really could be a helper function that is called within the setup() function itself, in which case it would be undefined.

from composition-api.

To me, in the context of the example it's pretty evident from the functions name that it's intended to be used as an event handler for a click event, which means ref would be available.

from composition-api.

Again, I think we are getting off track of the purpose of this issue (my fault).

We cannot (naturally) pass refs to custom extracted "hooks".

from composition-api.