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.

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

Misleading instructions at role prefer-property-order #58

Closed
moshest opened this issue Aug 25, 2017 · 6 comments
Closed

Misleading instructions at role prefer-property-order #58

moshest opened this issue Aug 25, 2017 · 6 comments
Assignees
Labels
enhancement 👑 New feature or request
Milestone

Comments

@moshest
Copy link

moshest commented Aug 25, 2017

My package.json look like this:

{
  "name": "foo",
  "version": "1.2.3",
  "main": "index.js",
  "repository": "https://github.com/foo/bar",
  "bugs": "https://github.com/foo/bar/issues"
}

With the default ordering bugs should be after version but the message I get is:

Your package.json properties are not in the desired order. Please move bugs before repository.

If I will put bugs before repository I will get:

Your package.json properties are not in the desired order. Please move bugs before main.

I think that better instructions will be (notice the quotes I added also):

Your package.json properties are not in the desired order. Please move "bugs" after "version".

My version is: 2.8.2

@tclindner
Copy link
Owner

Hi @moshest this is a bit tricky. We don’t require all properties in the list to be in the package.json file. Do you have any recommendations on how to approach this problem? I plan on looking at it more this week.

@moshest
Copy link
Author

moshest commented Aug 29, 2017

Yes, I think that should do it:

const propertiesOrder = ['name', 'version', ...];

const orderMap = new Map();
propertiesOrder.forEach((property, index) => {
  orderMap.set(property, index);
});

let i = 0;
Object.keys(packageJson).forEach((property, index) => {
  if (!orderMap.has(property)) return;
  
  const pos = orderMap.get(property);
  if (pos === i) {
    i += 1;
    return;
  }

  if (i > 0) {
    throw new Error(`Please move "${property}" after "${propertiesOrder[i - 1]}".`);
  } else {
    throw new Error(`Please move "${property}" to the top`);
  }
});

To solve #57, We should filter propertiesOrder first:

let propertiesOrder = ['name', 'version', ...];
propertiesOrder = propertiesOrder.filter(property => packageJson.hasOwnProperty(property));

@tclindner tclindner self-assigned this Aug 30, 2017
@tclindner tclindner added the enhancement 👑 New feature or request label Aug 30, 2017
@tclindner tclindner added this to the v2.10.0 milestone Aug 30, 2017
@tclindner
Copy link
Owner

Thanks for the feedback, @moshest!

tclindner pushed a commit that referenced this issue Sep 2, 2017
This change gives better recommendations for what change is required by
the user to resolve the lint issue. It also no longer throws an error
when a property exists in the package.json file that doesn't exist in
the preferred property order array. Thanks @moshest for the input.
tclindner added a commit that referenced this issue Sep 2, 2017
* Closes #57 and #58

This change gives better recommendations for what change is required by
the user to resolve the lint issue. It also no longer throws an error
when a property exists in the package.json file that doesn't exist in
the preferred property order array. Thanks @moshest for the input.

* Remove unused variable
@tclindner
Copy link
Owner

Hi @moshest v2.10.0 has been released! Thank you again.

@moshest
Copy link
Author

moshest commented Sep 2, 2017

It's works great!
A added a PR with a small fix the the error message (already tested on my packages).

@tclindner
Copy link
Owner

Awesome! 🥇

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement 👑 New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants