Skip to content

Add no-shared-component-data rule. #84

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
Merged
merged 3 commits into from
Jul 23, 2017
Merged

Add no-shared-component-data rule. #84

merged 3 commits into from
Jul 23, 2017

Conversation

@armano2
Copy link
Contributor

@armano2 armano2 commented Jul 16, 2017 •
edited
Loading

This PR implements rules proposed in #79

DONE:

  • Create documtation
  • Add tests
  • Implement rule

Importnant:

This rule splits executeOnVueComponent into executeOnVueInstance and executeOnVueComponent and adds alias executeOnVue

  • if you want to process only Vue components use utils.executeOnVueComponent
  • if you want to process only new Vue use utils.executeOnVueInstance
  • If you want to process both use utils.executeOnVue

@armano2 armano2 changed the title WIP: Add component-date rule. Add component-date rule. Jul 16, 2017
@armano2 armano2 changed the title Add component-date rule. Add component-data rule. Jul 16, 2017
@armano2 armano2 force-pushed the patch-8-component-data branch from 89f7ec3 to 8f4c945 Compare July 19, 2017 16:20
@armano2
Copy link
Contributor Author

armano2 commented Jul 19, 2017

@michalsnik cleaned up and merged

@mysticatea
Copy link
Member

mysticatea commented Jul 19, 2017

I'm considering the rule name. In general, eslint rules which disallow something starts with no-.

I don't have the best idea, but maybe no-shared-component-data?
I guess that it's shared by component instances if the data is not a function.

@armano2
Copy link
Contributor Author

armano2 commented Jul 19, 2017

@mysticatea no-shared-component-data make more sense than component-data, i will rename it

@armano2 armano2 changed the title Add component-data rule. Add no-shared-component-data rule. Jul 19, 2017
michalsnik
michalsnik reviewed Jul 21, 2017
View reviewed changes
Copy link
Member

@michalsnik michalsnik left a comment

Choose a reason for hiding this comment

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

Few minor suggestions @armano2

lib/rules/no-shared-component-data.js Outdated
).forEach(cp => {
context.report({
node: cp.value,
message: 'Component `data` must be a function.'
Copy link
Member

@michalsnik michalsnik Jul 21, 2017

Choose a reason for hiding this comment

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

What do you think about changing the message to:
data property in component must be a function

lib/rules/no-shared-component-data.js Outdated
module.exports = {
meta: {
docs: {
description: 'Enforces that a component data must be a function',
Copy link
Member

@michalsnik michalsnik Jul 21, 2017

Choose a reason for hiding this comment

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

component's

docs/rules/no-shared-component-data.md Outdated
@@ -0,0 +1,33 @@
# Enforces that a component data must be a function (no-shared-component-data)
Copy link
Member

@michalsnik michalsnik Jul 21, 2017

Choose a reason for hiding this comment

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

What about: Enforces component's data property to be a function ?

lib/rules/no-shared-component-data.js Outdated
@@ -0,0 +1,51 @@
/**
* @fileoverview When using the data property on a component (i.e. anywhere except on new Vue), the value must be a function that returns an object.
Copy link
Member

@michalsnik michalsnik Jul 21, 2017

Choose a reason for hiding this comment

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

I'd use the same title as in documentation

lib/rules/no-shared-component-data.js Outdated
p.key.name === 'data' &&
p.value.type !== 'FunctionExpression' &&
p.value.type !== 'Identifier'
).forEach(cp => {
Copy link
Member

@michalsnik michalsnik Jul 21, 2017

Choose a reason for hiding this comment

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

Move .forEach to new line for consistency

@armano2 armano2 force-pushed the patch-8-component-data branch from 5992614 to 7453a77 Compare July 22, 2017 12:29
@armano2
Copy link
Contributor Author

armano2 commented Jul 22, 2017 •
edited
Loading

@michalsnik suggestions applied + i rebased changes and applied executeOnVueInstance, executeOnVueComponent, executeOnVue to already merged code.

michalsnik
michalsnik suggested changes Jul 23, 2017
View reviewed changes
lib/rules/no-shared-component-data.js Outdated
// Public
// ----------------------------------------------------------------------

return Object.assign({},
Copy link
Member

@michalsnik michalsnik Jul 22, 2017

Choose a reason for hiding this comment

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

Object.assign seems to be redundant

@armano2
Copy link
Contributor Author

armano2 commented Jul 23, 2017

@michalsnik fixed

@michalsnik michalsnik merged commit 1ea1396 into vuejs:master Jul 23, 2017
@armano2 armano2 deleted the patch-8-component-data branch July 23, 2017 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@michalsnik michalsnik michalsnik approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

semver-minor

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@armano2 @mysticatea @michalsnik