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

Allow constraining ref attribute to not allow string when using tsx. #3455

Open
trainiac opened this issue Mar 22, 2021 · 9 comments · May be fixed by #5387
Open

Allow constraining ref attribute to not allow string when using tsx. #3455

trainiac opened this issue Mar 22, 2021 · 9 comments · May be fixed by #5387
Labels
has PR A pull request has already been submitted to solve the issue scope: types

Comments

@trainiac
Copy link

trainiac commented Mar 22, 2021

What problem does this feature solve?

When writing a component in tsx, passing a string to a ref attribute on a native element does not work and typescript does not tell you that anything is wrong.

This is not a problem with custom components because we can override ref typing like:

import * as RuntimeCore from '@vue/runtime-core'
declare module '@vue/runtime-core' {
  declare interface ComponentCustomProps {
    ref?:
      | RuntimeCore.Ref
      | ((ref: Element | RuntimeCore.ComponentInternalInstance | null) => void)
  }
}

What does the proposed API look like?

I'm not sure the best way to solve this problem but here is where the problem is:

// @vue/runtime-dom/dist

type ReservedProps = {
  key?: string | number
  ref?:
    | string
    | RuntimeCore.Ref
    | ((ref: Element | RuntimeCore.ComponentInternalInstance | null) => void)
}

// need to be able to override to say

type ReservedProps = {
  key?: string | number
  ref?:
    | RuntimeCore.Ref
    | ((ref: Element | RuntimeCore.ComponentInternalInstance | null) => void)
}
@HcySunYang
Copy link
Member

This should work:

defineComponent({
  setup() {
    const dom = ref(null)

    return {
      dom
    }
  },
  render() {
    return <div ref="dom"></div>
  }
})

what do you actually mean?

@HcySunYang HcySunYang added the need more info Further information is requested label Mar 22, 2021
@trainiac
Copy link
Author

trainiac commented Mar 22, 2021

Using jsx-next, recommended here, for writing tsx files and using this syntax.

import { defineComponent } from "vue";

defineComponent({
  setup() {
    const dom = ref(null)
    return () => (
      <div ref={dom} />
    );
  },
});

@HcySunYang
Copy link
Member

But according to the example I provided above, it allows specifying ref as a string

@trainiac
Copy link
Author

trainiac commented Mar 22, 2021

jsx-next provides two syntaxes for writing render functions. It does work the way you wrote it but it does not work the way I wrote it. I prefer returning a render function from setup as it's more succinct and typescript tells me when a custom component is not within the template scope. jsx-next offers a way to write it the way I like so I'd just like to be able to control what is allowed for ref values on native input elements. For those that choose this method for their whole application they should be able to restrict what is passed to ref to suit their style.

// Bulkier syntax
import { defineComponent } from "vue";
import CustomComponent from './CustomComponent'

defineComponent({
  components: {
    CustomComponent
  }
  setup() {
    const dom = ref(null)
    return {
      dom,
       title: 'info'
    }
  },
  render () {
      return  (
        <div ref="dom" >
          <CustomComponent title={this.title} />
        </div>
      );
  }
});
// more succinct syntax
import { defineComponent } from "vue";
import CustomComponent from './CustomComponent'

defineComponent({
  setup() {
    const dom = ref(null)
    const title = 'info'
    return () => (
      <div ref={dom}>
        <CustomComponent title={title} />
      </div>
    );
  },
});

@trainiac
Copy link
Author

trainiac commented Mar 22, 2021

To be clear this doesn't work and I'd like typescript to tell me what what is being passed to ref is an error

import { defineComponent } from "vue";
import CustomComponent from './CustomComponent'

defineComponent({
  setup() {
    const dom = ref(null)
    const title = 'info'
    return () => (
      <div ref="dom">
        <CustomComponent title={title} />
      </div>
    );
  },
});

@HcySunYang
Copy link
Member

I know what you mean: you mean that if jsx is used as the return value of setup then it is not allowed to be specified as a string, otherwise, it can. But we cannot restrict ref to not be able to specify a string, because it actually allows the use of string types. And I don’t think we can limit ref correctly unless we can analyze its context correctly.

@trainiac
Copy link
Author

trainiac commented Mar 22, 2021

I'm not saying that vue should limit it for everyone. I'm saying that consumers of vue should be able to customize it for their own purposes. For example, by adding this to my application type definition files I fix this issue for custom components, because it ends up overriding what is defined in @vue/runtime-dom.

import * as RuntimeCore from '@vue/runtime-core'
declare module '@vue/runtime-core' {
  declare interface ComponentCustomProps {
    ref?:
      | RuntimeCore.Ref
      | ((ref: Element | RuntimeCore.ComponentInternalInstance | null) => void)
  }
}

now typescript will hint that ref is incorrect below

import { defineComponent } from "vue";
import CustomComponent from './CustomComponent'

defineComponent({
  setup() {
    const customRef = ref(null)
    return () => (
        <CustomComponent ref="customRef" />
    );
  },
});

I'd like the same ability to define a type definition file in my application to override what is in @vue/runtime-dom so that for my application you cannot pass strings to a native element ref attribute.

@HcySunYang
Copy link
Member

@trainiac I made a PR #3458, but I’m not sure if it will eventually be accepted

@HcySunYang HcySunYang added scope: types and removed need more info Further information is requested labels Mar 22, 2021
@trainiac
Copy link
Author

Thank you @HcySunYang

@pikax pikax added the has PR A pull request has already been submitted to solve the issue label Oct 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has PR A pull request has already been submitted to solve the issue scope: types
Projects
None yet
3 participants