Skip to content
This repository has been archived by the owner on Jan 19, 2019. It is now read-only.

chore: update package dependencies (fixes #184) #186

Merged
merged 3 commits into from
Nov 28, 2018

Conversation

weirdpattern
Copy link
Collaborator

@weirdpattern weirdpattern commented Nov 27, 2018

Updates package dependencies and introduces eslint and typescript-eslint-parser as peer dependencies

Considerations:

  1. The PR includes minor lint changes to 3 rules introduced with the newer eslint version
  2. The PR bumps all versions to latest except: eslint-plugin-node (peer dependency of eslint-config-eslint), typescript and typescript-eslint-parser (these two being worked in Migrate typescript-eslint-parser to 21.0.2 #174)
  3. Peer dependencies are kind of loose (at least eslint), we can change that...

Copy link
Owner

@bradzacher bradzacher left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Owner

@bradzacher bradzacher left a comment

Choose a reason for hiding this comment

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

Actually - considering this is for 1.0.0

Could you please make the change described here:
eslint/eslint#11127 (comment)

It'll let us create a direct dependency on the parser, so we only "allow" people to use the tested version of it.
(also update the readme please :)

// eslint-plugin-foo/parser.js
module.exports = require('mylanguage-parser-eslint')

// eslint-plugin-foo
module.exports.rules = rules;
// .eslintrc
{
  "parser": "eslint-plugin-foo/parser",
  "plugins": [
    "foo"
  ]
}

@weirdpattern weirdpattern added this to the 1.0.0 milestone Nov 27, 2018
@weirdpattern
Copy link
Collaborator Author

weirdpattern commented Nov 27, 2018

Adding the 1.0.0 milestone based on this

Actually - considering this is for 1.0.0

This looks great, pretty sure is going to help reducing the number of false positives caused by using an unsupported parser.

I will take care of this tonight.

@bradzacher bradzacher changed the title chore: udpate package dependencies (fixes #184) chore: update package dependencies (fixes #184) Nov 28, 2018
Updates package dependencies and introduces eslint and typescript-eslint-parser as peer dependencies
@weirdpattern
Copy link
Collaborator Author

@bradzacher alright, this should be ready for one last code review and merge...

By the way, I'm seeing some odd behaviors after #182 was merged... I think the .vscode/settings.json eslint.validate autofix option is in direct conflict with prettier-eslint and eslint-docs... saving a file gives one format, running yarn format gives another, same for the eslint-docs script... this causes a lot of unnecessary LOC changes...

Good examples of this are:

  • The README.md for eslint-docs
  • The lib/rules/class-name-casing.js rule for prettier-eslint

I should probably create a bug for this... just want you to double check, maybe is something I'm doing wrong?

@bradzacher
Copy link
Owner

hmm. have you got the Prettier extension installed?
I may have been a bit eager chucking those config options in.

@weirdpattern
Copy link
Collaborator Author

Unless is not this one... then yes...
extensions

I will review my own settings tomorrow... but as far as I can tell... everything should be in order

@weirdpattern
Copy link
Collaborator Author

And I just updated the package.json with the lint-staged change that you just merged

@bradzacher
Copy link
Owner

Ah - you have beautify extension.
I found out the hard way that that extension directly conflicts with the prettier extension!

Either way, I've disabled automatic formatting in #192
I can see this frustrating a lot of people if they don't configure everything right.

@weirdpattern
Copy link
Collaborator Author

I was pretty sure it was something with my configuration... 😄

@weirdpattern weirdpattern merged commit 096e8ba into bradzacher:master Nov 28, 2018
@weirdpattern weirdpattern deleted the plugin-dependencies branch November 28, 2018 06:55
@armano2 armano2 mentioned this pull request Dec 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants