feat(reactivity-transform): allow custom shorthands #5856
Conversation
✅ Deploy Preview for vue-sfc-playground ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for vue-next-template-explorer ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
❌ Deploy Preview for vuejs-coverage failed.
|
| const shorthands = [...builtInShorthands, ...(options.shorthands || [])] | ||
| const transformCheckRE = new RegExp( | ||
| `[^\\w]\\$(?:\$|${shorthands.map(escapeRegExp).join('|')})?\\s*(\\(|\\<)` | ||
| ) |
| const { transform, transformAST, shouldTransform } = | ||
| /*#__PURE__*/ createReactivityTransformer() | ||
| export { transform, transformAST, shouldTransform } |
There was a problem hiding this comment.
change 3
To be compatible with the original top-level APIs
| function escapeRegExp(str: string) { | ||
| return str.replace(/[|\\{}()[\]^$+*?.]/g, '\\$&').replace(/-/g, '\\x2d') | ||
| } |
| ? options.reactivityTransform | ||
| : undefined | ||
| ) | ||
| : undefined |
There was a problem hiding this comment.
change 6
use the transformer instead of top-level apis, and allows reactivityTransform to be an option.
There was a problem hiding this comment.
I don't think the createReactivityTransformer implementation is necessary. I think the whole feature can simply be implemented by adding an extra argument to transformAST, (and as an extra option to the transform options object).
| // shorthands | ||
| const name = method.slice(1) | ||
| const isBuiltIn = builtInShorthands.includes(name) | ||
| if (id.type !== 'Identifier' && isBuiltIn) { | ||
| error(`${method}() cannot be used with destructure patterns.`, call) | ||
| } else { | ||
| // replace call | ||
| s.overwrite( | ||
| call.start! + offset, | ||
| call.start! + method.length + offset, | ||
| isBuiltIn ? helper(name) : name | ||
| ) | ||
| if (id.type === 'Identifier') { | ||
| // single variable | ||
| registerRefBinding(id) | ||
| } else if (id.type === 'ObjectPattern') { | ||
| processRefObjectPattern(id, call) | ||
| } else if (id.type === 'ArrayPattern') { | ||
| processRefArrayPattern(id, call) | ||
| } | ||
| } |
There was a problem hiding this comment.
change 2
treat differently for builtin shorthands and custom shorthands
| ? options.reactivityTransform | ||
| : undefined | ||
| ) | ||
| : undefined |
There was a problem hiding this comment.
I don't think the createReactivityTransformer implementation is necessary. I think the whole feature can simply be implemented by adding an extra argument to transformAST, (and as an extra option to the transform options object).
|
We built the regex here using the options core/packages/reactivity-transform/src/reactivityTransform.ts Lines 59 to 62 in af488c2 which also affects how I also exposed the top-level APIs to be compatible core/packages/reactivity-transform/src/reactivityTransform.ts Lines 707 to 709 in af488c2 In the long term, if we want to have more options, I still think the |
|
Ah I see, didn't notice the check regex is being dynamically generated now. |
|
There are a few conflicts with main, LGTM after resolve |
|
Done |
| error(`${method}() cannot be used with destructure patterns.`, call) | ||
| } else { |
There was a problem hiding this comment.
error will never return, we could remove the else branch.
Related issue in Nuxt 3:
Reactivity transform provides the awesome DX to work on reactive data at ease. While the built-in shorthands like
$refand$computedare quite convenient, when it comes to custom composable functions, the usage becomes a bit verbose like$(useMouse({ ... })). In high-level integrations like Nuxt or with vite plugins, we could provide auto import for common APIs and libraries with proper types, having custom shorthands$useMouse({ ... })would improve the DX even further.This PR enables the options to provide custom shorthands for the transformer with minimal features. It essential handles
$foo()->$(foo())while leaving the complexity of helper imports and type generation to the uplevel integrations.