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.

Sign up for GitHub

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

Jump to bottom

fix: support multiple macro calls in vue/define-macros-order #2386

Merged
merged 3 commits into from
Feb 1, 2024
Merged

fix: support multiple macro calls in vue/define-macros-order #2386

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5ce6a67
fix: support multiple macro calls in vue/define-macros-order
hendrikheil Jan 30, 2024
da93adc
tests: add invalid tests for multiple defineModel calls
hendrikheil Jan 31, 2024
d389cec
Update define-macros-order.js
hendrikheil Jan 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 13 additions & 7 deletions lib/rules/define-macros-order.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,27 +104,29 @@ function create(context) {
const order = (options[0] && options[0].order) || DEFAULT_ORDER
/** @type {boolean} */
const defineExposeLast = (options[0] && options[0].defineExposeLast) || false
/** @type {Map<string, ASTNode>} */
/** @type {Map<string, ASTNode[]>} */
const macrosNodes = new Map()
/** @type {ASTNode} */
let defineExposeNode

return utils.compositingVisitors(
utils.defineScriptSetupVisitor(context, {
onDefinePropsExit(node) {
macrosNodes.set(MACROS_PROPS, getDefineMacrosStatement(node))
macrosNodes.set(MACROS_PROPS, [getDefineMacrosStatement(node)])
},
onDefineEmitsExit(node) {
macrosNodes.set(MACROS_EMITS, getDefineMacrosStatement(node))
macrosNodes.set(MACROS_EMITS, [getDefineMacrosStatement(node)])
},
onDefineOptionsExit(node) {
macrosNodes.set(MACROS_OPTIONS, getDefineMacrosStatement(node))
macrosNodes.set(MACROS_OPTIONS, [getDefineMacrosStatement(node)])
},
onDefineSlotsExit(node) {
macrosNodes.set(MACROS_SLOTS, getDefineMacrosStatement(node))
macrosNodes.set(MACROS_SLOTS, [getDefineMacrosStatement(node)])
},
onDefineModelExit(node) {
macrosNodes.set(MACROS_MODEL, getDefineMacrosStatement(node))
const currentModelMacros = macrosNodes.get(MACROS_MODEL) ?? []
currentModelMacros.push(getDefineMacrosStatement(node))
macrosNodes.set(MACROS_MODEL, currentModelMacros)
},
onDefineExposeExit(node) {
defineExposeNode = getDefineMacrosStatement(node)
Expand All @@ -142,7 +144,10 @@ function create(context) {
program
)
const orderedList = order
.map((name) => ({ name, node: macrosNodes.get(name) }))
.flatMap((name) => {
const nodes = macrosNodes.get(name) ?? []
return nodes.map((node) => ({ name, node }))
})
.filter(
/** @returns {data is OrderedData} */
(data) => utils.isDef(data.node)
Expand All @@ -165,6 +170,7 @@ function create(context) {
.map(({ node }) => node)
const targetStatementIndex =
moveTargetNodes.indexOf(targetStatement)

if (targetStatementIndex >= 0) {
moveTargetNodes = moveTargetNodes.slice(0, targetStatementIndex)
}
Expand Down
113 changes: 113 additions & 0 deletions tests/lib/rules/define-macros-order.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,22 @@ tester.run('define-macros-order', rule, {
parser: require.resolve('@typescript-eslint/parser')
}
}
},
{
filename: 'test.vue',
code: `
<script setup>
const first = defineModel('first')
const second = defineModel('second')

const slots = defineSlots()
</script>
`,
options: [
{
order: ['defineModel', 'defineSlots']
}
]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this PR!
Could you also add invalid test cases with multiple defineModels?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a couple I could think of. Also one that tests that order of occurrence is maintained and not re-ordered like I suggested in the initial issue

Can you think of any other interesting cases to check?

}
],
invalid: [
Expand Down Expand Up @@ -818,6 +834,103 @@ tester.run('define-macros-order', rule, {
line: 8
}
]
},
{
filename: 'test.vue',
code: `
<script setup>
/** options */
defineOptions({})
/** model */
const first = defineModel('first')
const second = defineModel('second')
</script>
`,
output: `
<script setup>
/** model */
const first = defineModel('first')
const second = defineModel('second')
/** options */
defineOptions({})
</script>
`,
options: [
{
order: ['defineModel', 'defineOptions']
}
],
errors: [
{
message: message('defineModel'),
line: 6
}
]
},
{
filename: 'test.vue',
code: `
<script setup>
const first = defineModel('first')
defineOptions({})
const second = defineModel('second')
</script>
`,
output: `
<script setup>
const first = defineModel('first')
const second = defineModel('second')
defineOptions({})
</script>
`,
options: [
{
order: ['defineModel', 'defineOptions']
}
],
errors: [
{
message: message('defineModel'),
line: 5
}
]
},
{
filename: 'test.vue',
code: `
<script setup>
const a = defineModel('a')
defineOptions({})
const c = defineModel('c')
defineExpose({})
const b = defineModel('b')
</script>
`,
output: `
<script setup>
const a = defineModel('a')
const c = defineModel('c')
const b = defineModel('b')
defineOptions({})
defineExpose({})
</script>
`,
options: [
{
order: ['defineModel', 'defineOptions'],
defineExposeLast: true
}
],
errors: [
{
message: message('defineModel'),
line: 5
},
{
message: defineExposeNotTheLast,
line: 6
}
]
}
]
})