-
-
Notifications
You must be signed in to change notification settings - Fork 681
chore: Keep Nuxt's 'asyncData' and 'fetch' with 'data' #823
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
Conversation
Change the order of asyncData and fetch properties to be next to data property. All those are function-wise more or less equivalent so IMO it makes sense to keep them together. asyncData and fetch are primarily for setting up component's data.
lib/rules/order-in-components.js
Outdated
@@ -19,11 +19,9 @@ const defaultOrder = [ | |||
'inheritAttrs', | |||
'model', | |||
['props', 'propsData'], | |||
'data', | |||
['asyncData', 'fetch', 'data'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest moving fetch to the top of asyncData
because it is more context less
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pi0 but as I understand, having asyncData and fetch in same array means the order can be freely chosen between those. Then it shouldn't be need for moving them around?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I have actually updated to have them in explicit order.
Ping |
@rchl I forgot: could we add |
Added after props and before data related props. Scream if you disagree. |
@rchl For me it's ok. Anyway, would it make sense at the end, as you can use methods & stuff in there too? |
True. How about:
At the very end would work for me too but this possibly makes a bit more sense. |
@rchl Agree with the proposed positioning |
Seems ready to merge? |
Merge please. :) |
cc @michalsnik @mysticatea 😌 |
Hey @rchl, thank you for your time and effort spent on this PR, contributions like yours help make Vue better for everyone. Cheers! 💚 |
OK, this messed up my code. lol /Users/vinayak/Development/Work/frontend/pages/signup.vue
101:3 warning The "methods" property should be above the "head" property on line 90 vue/order-in-components
✖ 145 problems (0 errors, 145 warnings) |
@vinayakkulkarni it's unfortunate that this fix was lingering for so long. You have an option to either override the rule in your eslint configuration or fix automatically with eslint --fix |
Change the order of asyncData and fetch properties to be next
to data property. All those are function-wise more or less
equivalent so IMO it makes sense to keep them together. asyncData
and fetch are primarily for setting up component's data.
@manniL