Skip to content

feat: support directives #179

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

Merged
merged 6 commits into from
Oct 18, 2021
Merged

feat: support directives #179

merged 6 commits into from
Oct 18, 2021

Conversation

sxzz
Copy link
Member

@sxzz sxzz commented Oct 13, 2021

close #155

@sxzz sxzz requested a review from antfu as a code owner October 13, 2021 07:29
@antfu
Copy link
Member

antfu commented Oct 13, 2021

Thanks for the hard work! However, I don't really want to introduce Babel here, giving the fact that for components it's quite straightforward to search for _resolveComponent usage without AST. Could we do the same for directives? I am not familiar with it but IIRC it's something like _withDirective?

@sxzz
Copy link
Member Author

sxzz commented Oct 13, 2021

Yes. But in Vue 2, it will be very hard to transform without AST.

var render = function () {
  var _vm = this
  var _h = _vm.$createElement
  var _c = _vm._self._c || _h
  return _c("test-comp", {
    directives: [
      { name: "loading", rawName: "v-loading", value: 123, expression: "123" }
    ]
  })
}

@antfu
Copy link
Member

antfu commented Oct 13, 2021

But honestly, do we really need this? I found directives are rarely used for me. For the few of them, I think it's better to register globally - I don't expect directives to be huge that impacting the code-splitting.

@sxzz
Copy link
Member Author

sxzz commented Oct 13, 2021

IMO, It can be used out of the box in some ui frameworks (element-plus, element-ui), if with this feature.

element-plus/element-plus#3776 (comment)

@antfu
Copy link
Member

antfu commented Oct 13, 2021

I am fine to have this but I don't want to introduce Babel that affecting the existing users just because of a relatively rarely used feature. If you absolutely need this, I think we should keep the original transformation, and have babel enabled only for Vue 2 directives with an explicit flag to enable (default to false).

@sxzz
Copy link
Member Author

sxzz commented Oct 13, 2021

OK. I will change it.

@sxzz
Copy link
Member Author

sxzz commented Oct 13, 2021

@antfu Done.

@sxzz sxzz force-pushed the feat/directive branch 2 times, most recently from 9106b0e to ecbaa3d Compare October 13, 2021 10:49
@sxzz
Copy link
Member Author

sxzz commented Oct 13, 2021

@antfu I fixed unit test. Feel free to tell me if you have any questions.

@sxzz
Copy link
Member Author

sxzz commented Oct 16, 2021

Hi, when can it gets merged?

@sxzz sxzz requested a review from antfu October 16, 2021 14:29
@sxzz sxzz merged commit 1328be1 into main Oct 18, 2021
@sxzz sxzz deleted the feat/directive branch October 18, 2021 22:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Option to import directives in UI resolver?
2 participants