Skip to content

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?

3 participants