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

Add Reactivity Transform macros #1799

Closed
wants to merge 1 commit into from
Closed

Add Reactivity Transform macros #1799

wants to merge 1 commit into from

Conversation

AndreasNasman
Copy link

@AndreasNasman AndreasNasman commented Feb 12, 2022

Closes #1798


I noticed that the new Reactivity Transform macros were missing in the vue/setup-compiler-macros option, resulting in e.g. '$ref' is not defined no-undef errors when not explicitly importing the macros.

I hope this implementation is correct. I looked at #1685 as an example.

@ota-meshi
Copy link
Member

Thank you for opening this PR.
However, this package does not support experimental features.
Because if this package tries to support experimental features, it will have to do a major version upgrade every time the RFC changes such as rename $ref. Because it's a breaking change for this package.

After the Reactivity Transform RFCs have been merged, we will work to support it.

@AndreasNasman
Copy link
Author

Ok, thanks for the clarification!

@AndreasNasman AndreasNasman deleted the add-reactivity-transform-macros branch February 18, 2022 11:27
@martinszeltins
Copy link

Thank you for opening this PR. However, this package does not support experimental features. Because if this package tries to support experimental features, it will have to do a major version upgrade every time the RFC changes such as rename $ref. Because it's a breaking change for this package.

After the Reactivity Transform RFCs have been merged, we will work to support it.

@ota-meshi But I am getting an error ESLint: 'id' is never reassigned. Use 'const' instead.(prefer-const)

I don't want to turn this rule off completely, what can I do?

@FloEdelmann
Copy link
Member

This is a different ESLint rule: https://eslint.org/docs/rules/prefer-const

Please use const id = … instead of let id = … or var id = ….

@martinszeltins
Copy link

This is a different ESLint rule: eslint.org/docs/rules/prefer-const

Please use const id = … instead of let id = … or var id = ….

Vue Reactivity Transform actually requires that you use let like this let id = $ref('') instead of const

@FloEdelmann
Copy link
Member

Are you sure? Most of the examples on the linked page do use let, but it is never explicitly mentioned that const is not allowed, and I don't see a reason why they shouldn't. Of course, if you're planning to use id++ or something else, you need let, but then prefer-const shouldn't complain either.

Note that I also found this related comment in the RFC: vuejs/rfcs#369 (reply in thread)

@AndreasNasman
Copy link
Author

You can use either let or const. The problem is that the JS you write isn't the same after the Vue compiler has processed it:

// Written input
let id = $ref(0)
const age = $ref(0)
id++ // 1
age++ // 1

// is actually (simplified):

// Compiled output
let id = { value: 0 }
const age = { value: 0 }
id.value++ // 1
age.value++ // 1

ESLint doesn't know the output of the Vue compiler, it can only analyze the code you write. You'd probably have to modify the prefer-const rule to ignore x = $ref cases and other Reactivity Transform macros.

@FloEdelmann
Copy link
Member

At least in your example, both let and const would work in the compiled output, since only the .value part is mutable and the variable itself is constant. So it doesn't really matter what you write in the input, does it? So you could simply use const whenever the prefer-const rule suggests to do so, and let otherwise`.

Modifying the core ESLint prefer-const rule to ignore Vue-specific reactivity macros will likely not happen. So the only option is to disable that rule and maybe (when reactivity macros are not experimental anymore) add a new rule in eslint-plugin-vue that wraps the core rule but ignores reactivity macros.

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.

Reactivity Transform macros missing
4 participants