Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Add runOutsideVue option to vue/sort-keys#1866

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Closed
lukethompson wants to merge3 commits intovuejs:masterfromlukethompson:sort-keys-run-outside-vue
Closed

Add runOutsideVue option to vue/sort-keys#1866

lukethompson wants to merge3 commits intovuejs:masterfromlukethompson:sort-keys-run-outside-vue

Conversation

lukethompson
Copy link

@lukethompsonlukethompson commentedApr 23, 2022
edited
Loading

Address issue raised here#1865

Updatevue/sort-keys to use existingoptions.runOutsideVue option, it seems like previously this option did not do anything.

If this change gets accepted we should update the docs to state which version this option will be available from 😄

Copy link
Member

@FloEdelmannFloEdelmann left a comment

Choose a reason for hiding this comment

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

Looks good! Interesting that the option existed before but was unused and undocumented.

I don't think we need to specify in the docs the version in which this "new" option is released, since it wasn't documented before.

Comment on lines 523 to 527
return {
c: 3,
a: 1,
b: 2
}
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!
Why is this not reported? I thinka,b, andc are Vue component properties and should be sorted.

Copy link
Author

Choose a reason for hiding this comment

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

Thank you for the review.

I have pushed a change that will ensure objects returned by thedata() method are sorted.

Just clarifying here, we only want to sort the keys ofdata() objects and we can leave objects defined withincomputed,watchers andmethods unsorted?

If that is the case then this is all good.

If we want to also sort all new objects defined within a vue instance but not directly attached to the component maybe we should look towards implementing a new option that only sorts keys that are directly attached to the vue instance.

For example

...computed: {  // I would like the computed keys to be sorted.  a () {    // But I do not want to sort new objects defined within my computed.     return {       b: null,       a: null,     }  },}

Copy link
Member

Choose a reason for hiding this comment

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

Sorry for late reply.

I think this rule should sort the keys of all objects that aren't reserved as Vue component definitions.
Therefore, I think the followingb anda should also be sorted. Because they areinside the Vue component.

computed: {  a () {     return {       b: null,       a: null,     }  },}

If you want to sort only the properties of Vue components (e.g. props, data, computed, watch, and methods), I think we can add another rule to exclude it from being checked by thevue/sort-keys rule.
I think it would be better to have a separate rule for sorting related to Vue components thanvue/sort-keys rule, likevue/order-in-components rule.
https://eslint.vuejs.org/rules/order-in-components.html
What do you think?

Copy link
Author

Choose a reason for hiding this comment

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

Hello, sorry for the slow reply, been very busy with work lately.

This rule already accepts anignoreGrandchildrenOf option that might be applicable here, iirc it only considers grandchildren keys of the same object, not newly created objects. Let me confirm this behaviour when I get a chance and maybe we could look towards making that ignore all grandchildren, not just grandchildren that are directly attached to the grandparent key.

Copy link
Member

Choose a reason for hiding this comment

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

The ignoreGrandchildrenOf option is an option to avoid conflicting with ordering conventions with Vue components.
If we need an option to ignore all grandchildren, I don't think that option should ignore the outside of the Vue component.

I thinkvue/sort-keys rule should have a role to enforce an order other than the Vue component ordering convention.
I think the order of Vue properties is the ordering convention of Vue components, so I think we need another rule.

I thinkrunOutsideVue=false is just an option to ignore the outside of Vue. (However, I'm not sure if it's necessary for the user.)

I think it should be another rule if you want to enforce the order of Vue properties.
Where do you want to enforce the order?

Copy link
Author

Choose a reason for hiding this comment

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

@ota-meshi I was hoping to enforce order on allprops,computed props,methods andwatch child keys, eg.

export default {  // These keys would not be enforced by this rule/option combination.  name: 'ComponentName',  props: {    // These keys order would be enforced by this rule/option combination    a: { type: String, default: null },    b: { type: String, default: null },  },    computed: {    c () {      return {        // These keys would not be enforced by this rule/option combination.        e: null,        d: null,      }    }  },}

I agree, I believe a new rule would be more appropriate for this. I have closed this PR for now and if I get a chance in the future I will submit a new PR, thanks.

@FloEdelmannFloEdelmann linked an issueMay 7, 2022 that may beclosed by this pull request
@ota-meshiota-meshi marked this pull request as draftAugust 23, 2022 11:05
@FloEdelmann
Copy link
Member

Ping@lukethompson

@lukethompson
Copy link
Author

Ping@lukethompson

Sorry@FloEdelmann – I have been too busy to pick this one back up. Will close the PR for now and re-open when I get a chance to work on this again 😃

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@FloEdelmannFloEdelmannFloEdelmann approved these changes

@Mighty-TulipMighty-TulipMighty-Tulip approved these changes

@ota-meshiota-meshiAwaiting requested review from ota-meshi

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

vue/sort-keys runOutsideVue option
4 participants
@lukethompson@FloEdelmann@ota-meshi@Mighty-Tulip

[8]ページ先頭

©2009-2025 Movatter.jp