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

Refactor the assets resolution of the script setup #3546

Closed
wants to merge 3 commits into from
Closed

Refactor the assets resolution of the script setup #3546

wants to merge 3 commits into from

Conversation

HcySunYang
Copy link
Member

Close: #3543

with <script setup>, Assets(components/directives) will be tried to resolve from the setup state, this leads to naming conflicts between assets and ordinary variables, e.g.

<template>
  <div v-focus/>
</template>

<script setup>
const focus = ref(0);
</script>

The constant focus in setup will overwrite the globally registered directive with the same name. This is because the above code will be compiled as:

export function render(_ctx, _cache, $props, $setup) {
  return _withDirectives((_openBlock(), _createBlock('div', null, null, 512 /* NEED_PATCH */)), [
    // Will no longer perform asset resolution
    [$setup["focus"]]
  ])
}

with $setup["focus"], it will no longer perform asset resolution.

My idea is that in order to avoid name conflicts, we should always perform asset resolution, which makes globally registered assets have a higher priority, and at the same time, use setup state as a fallback. E.g.

export function render(_ctx, _cache, $props, $setup) {
  const _directive_focus = _resolveDirective(
    // resolve globally registered assets
    "focus",
    // warnMissing
    false,
    // fallback, when the asset resolution fails, use the fallback instead
    $setup["focus"]
  )

  return _withDirectives((_openBlock(), _createBlock('div', null, null, 512 /* NEED_PATCH */)), [
    [_directive_focus]
  ])
}

This behavior also applies to components resolution. And it only takes effect when there is a state with the same name in the setup result, without breaking anything.

@HcySunYang HcySunYang added the 🔨 p3-minor-bug Priority 3: this fixes a bug, but is an edge case that only affects very specific usage. label Apr 6, 2021
@yyx990803
Copy link
Member

I don't think we should prioritize global asset registrations over local variable resolutions. Local resolution should always have higher priority.

To reduce the potential shadowing of directives (which is way less obvious and more likely than components), I reverted it to require the variable to have a v prefix (RFC updates soon) in d35e0b1.

@yyx990803 yyx990803 closed this Jun 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔨 p3-minor-bug Priority 3: this fixes a bug, but is an edge case that only affects very specific usage. scope: script-setup
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Script Setup] Global directives collide with component's local vars
3 participants