No extraneous dependencies, also documentation improvements #11
Conversation
boutell
requested review from
bgantick,
abea,
austinstarin,
plantainrain,
grdunn and
ngranahan
boutell
changed the title
|
The "no extraneous dependencies" rule detects the situation where your project is depending on modules it doesn't have listed in package.json, and you are skating by because one of your other dependencies "just happens" to depend on that package... for now. I think this one is highly valuable because a nested dependency like that can go away at any moment, like the lodash issue we had with a few client projects and apostrophe. This is a really easy mistake to make — I added it because today someone pointed out that one newer file in apostrophe still depended on |
|
(Apostrophe has its own eslint package now, but I thought this was worth contributing back because it's going to be frequent with client projects too.) |
README.md
Outdated
| @@ -1,6 +1,6 @@ | |||
| ## Install | |||
|
|
|||
| This module is for advanced users looking for a shareable configuration across projects for ESLint. | |||
| This module is standard in all P'unk Avenue apostrophe projects. It provides warnings of potential bugs and errors in your code. It is recommended for developers of all skill levels working with Apostrophe. | |||
There was a problem hiding this comment.
ApostropheCMS instead of "apostrophe"
|
This doesn't seem to be a stock eslint rule. Is there a plugin required for this @boutell ? |
|
Ah, I see it's part of |
|
Actually I left out a new requirement in the docs, eslint-plugin-import.
Unfortunately, eslint is not smart enough to load the deps of an eslint
package from the eslint package, they all have to be installed in the
project. I checked. There's an issue about addressing it which seems to be
making progress.
But in the meantime this means that projects will need to npm install
eslint-plugin-import when they update eslint-config-punkave after this
change.
…On Wed, Jul 11, 2018 at 9:22 AM, Brian Gantick ***@***.***> wrote:
Ah, I see it's part of eslint-config-standard - should we add all of our
deps to package.json?: eslint-plugin-promise eslint-plugin-standard
eslint-plugin-react eslint-config-standard eslint-config-punkave
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#11 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAB9fTIpXtWodNTIttVhvLlmMK6jzwlcks5uFfwKgaJpZM4VLF5M>
.
--
*THOMAS BOUTELL, CHIEF SOFTWARE ARCHITECT*
P'UNK AVENUE | (215) 755-1330 | punkave.com
|
|
Gotcha, in that case does |
|
It is?
If it is, great. It's an explicit dependency in apostrophe so I wonder. I
thought this was simple but it clearly needs trying out in context. No good
deed heh
…On Wed, Jul 11, 2018 at 9:45 AM, Brian Gantick ***@***.***> wrote:
Gotcha, in that case does eslint-plugin-import not need to be installed
since it's part of eslint-plugin-standard?
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#11 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AROF6La7ZtpS4Gp2HkOlQiTdzeOHcmhjks5uFgFwgaJpZM4VLF5M>
.
--
*THOMAS BOUTELL, CHIEF SOFTWARE ARCHITECT*
P'UNK AVENUE | (215) 755-1330 | punkave.com
|
|
It's in the dev deps: https://github.com/standard/eslint-config-standard/blob/master/package.json#L15 |
|
I think I'd better try this as documented in a real project.
…On Wed, Jul 11, 2018 at 10:03 AM, Brian Gantick ***@***.***> wrote:
It's in the dev deps: https://github.com/standard/
eslint-config-standard/blob/master/package.json#L15
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#11 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAB9ff6pSzSRxltifWIB2Lv-i7Gu5ghZks5uFgWsgaJpZM4VLF5M>
.
--
*THOMAS BOUTELL, CHIEF SOFTWARE ARCHITECT*
P'UNK AVENUE | (215) 755-1330 | punkave.com
|
|
I tested this. It works fine in a real project because our client-boilerplate already depends on eslint-plugin-import. @austinstarin @abea there's been a bunch of discussion, I think matters are resolved, I still see "requested changes" from you guys. Can you update your reviews or let me know what is still actionable? Thanks! |
|
|
||
| In the project root directory run: | ||
|
|
||
| ```bash | ||
| npm install eslint eslint-plugin-promise eslint-plugin-standard eslint-plugin-react eslint-config-standard eslint-config-punkave --save-dev | ||
| npm install eslint eslint-plugin-promise eslint-plugin-import |
No description provided.