<script setup> used alongside Options API can break ref in production mode

[adamsol]

Vue version

3.2.37

Link to minimal reproduction

https://sfc.vuejs.org/#eNp9UNtOwzAM/ZXITyCtibQhIZUOjf/IS2k9Wlguip0Nqeq/k6wFuk2an+Jz8XE8wJv38hgRStC2YjT+UDO+aitSVb31kUXA/VYDYR2aToNQia3UQppaakLvWRBy9LO5cZZYnLqkOWIQW/G0fsnGSbq0zQb89i6waHFfxwOLYUJzGeTOtVQusVzTSg+PYhhX/8zve7yMgxX0JicUpvbyk5xNfz4P1DNBGv4iNKSj5F5Dx+ypVCpa//UhG2fULnEqRMu9waJ1ZreRa7l5Vm1PvMQlkinegzsRhpSoYd7sPFwlMB2mCGhbDBjuhl1pLwKvuJvQnDlqO8L4Awhro+k=

Steps to reproduce

No additional steps required.

What is expected?

Input to be displayed and no error.

What is actually happening?

ReferenceError: search is not defined

System Info

No response

Any additional comments?

The error occurs only when the ref is named like one of the component's variables/methods.
The error occurs only in production mode.
There is no error if there is nothing in the <script setup> block.

[LinusBorg]

This is kind of expected. It's strongly discouraged to mix script setup with Options API like that.

In the docs we explain that a second script block should be used for:

• Declare options that cannot be expressed in <script setup>, for example inheritAttrs or custom options enabled via plugins.

However we can do better here:

1. Make it clear in the docs that using Options APIs that can be expressed in Script setup (like methods) is strongly discouraged and can cause unwanted side-effects or bugs (like this one).
2. additionally we could log a warning during development when such an option is being used in the second script blog. Throwing an error would be cleaner but technically be a breaking change.

[LinusBorg]

/cc @skirtles-code What do you think about this in terms of docs, expected behavior and such?

[adamsol]

It's a pity, I hoped I could use <script setup> to import child components without having to redeclare them via Options API. The documentation saying:

<script setup> can be used alongside normal <script>. A normal <script> may be needed in cases where we need to: [...]

certainly doesn't suggest that it's strongly discouraged to do so.

[skirtles-code]

/cc @skirtles-code What do you think about this in terms of docs, expected behavior and such?

We probably can tighten this up a bit in the docs, but I also feel like there genuinely is a core bug here.

The ref: search part in the compiled output is only being included if some very specific criteria are being met. I'm not familiar with that part of the core code, but I think it would be worth identifying exactly why having a method with the same name as the ref triggers this problem. It feels like that same flawed detection logic could potentially be used elsewhere.

I agree that having some more compiler warnings would be a good idea for the options that can cause conflicts. Off the top of my head: setup, render, template, props, emits, expose. But I personally don't think that having a methods section should be considered a warning. I think that should just work, as it currently does with a development build.

Likewise for #6241. In my opinion that's a bug that should be fixed unless there's a good technical reason why it can't be made to work.

I'm not suggesting that combining <script setup> and <script> like this should be encouraged, but I think we're effectively discussing forbidding it for all but a handful of options, and to me that seems a bit too extreme. I personally would have expected the code examples on these two issues to have worked in production builds just like they do in development.

[LinusBorg]

Good points, I was a bit too quick to judge here, thanks!

Reopened #6241 accordingly. The generated code in that issue's playground even already uses the render proxy correctly to read ctx.query for the v-model value, but doesn't do so for the generated event listener. That seems fixable.

In this issue here concerning the ref, we might be able to find a better way to handle it, I'll leave that to the folks who really know their way around our compiler internals.

That being said, I also still think we should very heavily discourage this. It goes totally against the intented use of script setup and a second script block. Getting this to work in all possible scenarios (presumably) adds lots of further complexity in the compiler for a niche use case, and I would expect use to come across more of these kinds of problems which we will have to fix one by one, for a use case we never really intended. Seems like a can of worms.

[aborn]

/cc @skirtles-code What do you think about this in terms of docs, expected behavior and such?

We probably can tighten this up a bit in the docs, but I also feel like there genuinely is a core bug here.

The ref: search part in the compiled output is only being included if some very specific criteria are being met. I'm not familiar with that part of the core code, but I think it would be worth identifying exactly why having a method with the same name as the ref triggers this problem. It feels like that same flawed detection logic could potentially be used elsewhere.

I agree that having some more compiler warnings would be a good idea for the options that can cause conflicts. Off the top of my head: setup, render, template, props, emits, expose. But I personally don't think that having a methods section should be considered a warning. I think that should just work, as it currently does with a development build.

Likewise for #6241. In my opinion that's a bug that should be fixed unless there's a good technical reason why it can't be made to work.

I'm not suggesting that combining <script setup> and <script> like this should be encouraged, but I think we're effectively discussing forbidding it for all but a handful of options, and to me that seems a bit too extreme. I personally would have expected the code examples on these two issues to have worked in production builds just like they do in development.

This what we expected. The different behavior between dev env and prod env makes us confusing.

[yyx990803]

This is fixed by 2c27556