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

Multiple globally installed presets and plugins don’t work #165

Open
pvdlg opened this issue Dec 3, 2017 · 39 comments
Open

Multiple globally installed presets and plugins don’t work #165

pvdlg opened this issue Dec 3, 2017 · 39 comments
Labels
help wanted 🙏 This could use your insight or help 👀 no/external This makes more sense somewhere else 🧑 semver/major This is a change 🐛 type/bug This is a problem 🙆 yes/confirmed This is confirmed and ready to be worked on

Comments

@pvdlg
Copy link

pvdlg commented Dec 3, 2017

Following the example in the README.md.

// example.md
* Hello

[World][]
// .remarkrc
{
  "plugins": [
   "remark-preset-lint-recommended",
   ["remark-lint-list-item-indent", false]
  ]
}
$ remark . 
example.md
       1:3  warning  Incorrect list-item indent: add 2 spaces  list-item-indent         remark-lint
  3:1-3:10  warning  Found reference to undefined definition   no-undefined-references  remark-lint

The list-item-indent rule should have been disabled, but it's still active.

@wooorm
Copy link
Member

wooorm commented Dec 3, 2017

Could you rub with DEBUG="*" remark . and post the results?

This problem can occur if remark-preset-lint-recommended loads a different remark-lint-list-item-indent (such as if versions mismatch).

@wooorm wooorm added the 🙉 open/needs-info This needs some more info label Dec 3, 2017
@pvdlg
Copy link
Author

pvdlg commented Dec 3, 2017

I create a gist with the logs and the .remarkrc: https://gist.github.com/pvdlg/b1eca9123b975ee66dd95cbde1aa134c

In this example I was trying to disable remark-lint-maximum-line-length.

This problem can occur if remark-preset-lint-recommended loads a different remark-lint-list-item-indent (such as if versions mismatch).

I think this situation can happen very frequently as npm and yarn tries to flatten the dependency tree.

So when remark-preset-lint-recommended specifies a dependency to remark-lint-list-item-indent it's entirely possible that there is something else in the dependency tree that also depends on remark-lint-list-item-indent and you might end up with this situation.

Wouldn't it be better to depend on module/rule name rather than Object identity?

@wooorm
Copy link
Member

wooorm commented Dec 3, 2017

The configuration you posted in the gist in different from what you posted above. Which one is this issue about?

Red flattening: that's what enables duping on object equality, otherwise it wouldn't be possible at all!

@wooorm
Copy link
Member

wooorm commented Dec 3, 2017

Where is your configuration file compared to the readme file you are processing? The logs show that it isn't found? What are you using to run remark?

@pvdlg
Copy link
Author

pvdlg commented Dec 3, 2017

I deleted the reproduction env I created yesterday and I posted a similar case instead. Sorry for the confusion.

Here is the gist with the case initially mentioned: https://gist.github.com/pvdlg/1ad2570fa203a1270f1b9e66d1930bff

The file structure is:

.
+-- test_remark/
|   +-- .remarkrc
|   +-- example.md

The logs show that it isn't found? What are you using to run remark?

I use the CLI remark . which is installed with npm -g

@wooorm
Copy link
Member

wooorm commented Dec 3, 2017

Hmm, how weird! What are the versions of node, npm, remark-lint, the preset, and the rule?
Can you check if the rule your turning off is deduped? Is it there one? Or is there one in the preset as well? Does rm -rf node_modules fix anything?

@pvdlg
Copy link
Author

pvdlg commented Dec 3, 2017

The new gist I posted also has the error unified-engine:find-up No files found.

Even thoughremark finds 2 warnings for list-item-indent and no-undefined-references so I guess the configuration is read...

@wooorm
Copy link
Member

wooorm commented Dec 3, 2017

Yeah, the configuration is read, I misread the logs as saying otherwise as well, but a later line shows its reading the configuration file anyway!

@pvdlg
Copy link
Author

pvdlg commented Dec 3, 2017

  • npm: 5.5.1
  • node: v8.9.1
$ npm ls -g remark-lint                                                                                                                       23ms 
/Users/vanduynslager/.npm-packages/lib
├── [email protected] 
├─┬ [email protected]
│ └── [email protected] 
├─┬ [email protected]
│ └── [email protected] 
└─┬ [email protected]
  └── [email protected] 
$ npm ls -g remark                                                                                                                        8s 672ms 
/Users/vanduynslager/.npm-packages/lib
├── [email protected] 
├─┬ [email protected]
│ └── [email protected] 
├─┬ [email protected]
│ └── [email protected] 
└─┬ [email protected]
  └─┬ [email protected]
    └── [email protected] 
$ npm ls -g remark-preset-lint-recommended                                                                                                7s 958ms 
/Users/vanduynslager/.npm-packages/lib
└── [email protected]
$ npm ls -g remark-lint-list-item-indent                                                                                                  8s 118ms 
/Users/vanduynslager/.npm-packages/lib
├─┬ [email protected]
│ └── [email protected] 
└─┬ [email protected]
  └── [email protected] 

@pvdlg
Copy link
Author

pvdlg commented Dec 3, 2017

Does rm -rf node_modules fix anything?

I could try. But I might loose the reproduction env. Plus I'd rather not have to delete my global node_modules...

@wooorm
Copy link
Member

wooorm commented Dec 3, 2017

Ohhh, I thought stuff was local. I’m guessing global mode interferes with the normal state of things.

Why are all things global?

@pvdlg
Copy link
Author

pvdlg commented Dec 3, 2017

Why are all things global?

Because I'd like to lint any markdown files including:

  • ones that in repo that I contribute to that doesn't include remark-lint in their package.json
  • ones that that are unrelated to any project / repo. For example when I write some markdown in a file and copy/paste it to a blog post
  • ones that are in non npm project (like a README.md in Java project)

@pvdlg
Copy link
Author

pvdlg commented Dec 3, 2017

That said, even in a local install you can't predict that you won't have two version of the same package coexisting. npm or yarn flatten the tree in some circumstances but not necessarly. Plus each new version of npm or yarn can change the flattening algorithm and produce different results.

This is why I proposed to rely on module/rule name rather than on object identity to dedupe.

@wooorm
Copy link
Member

wooorm commented Dec 3, 2017

There’s no good way of doing this, I think, but you’re the first to raise an issue about it after it existing for a while. And probably because of global installs!

Why not create a .remarkrc in ~, and install everything except remark-cli there? remark-cli will always look for and load ~/.remarkrc if it exists (or a closer one if there is one)!

@pvdlg
Copy link
Author

pvdlg commented Dec 3, 2017

Why not create a .remarkrc in ~, and install everything except remark-cli there? remark-cli will always look for and load ~/.remarkrc if it exists (or a closer one if there is one)!

This is more or less my config. Everything is installed globally and I put .remarkrc in ~.

Are you suggesting to have a package.json in ~ and run npm install from there?

@wooorm
Copy link
Member

wooorm commented Dec 3, 2017

Yes, or even point from your config file to plugins in some other directory:

// ~/.remarkrc
{
  "plugins": [
    "./some/place/node_modules/remark-preset-lint-recommended",
    ["./some/place/node_modules/remark-lint-list-item-indent", false] // a
    ["./some/place/node_modules/remark-preset-lint-recommended/node_modules/remark-lint-list-item-indent", false] // or b
  ]
}

@pvdlg
Copy link
Author

pvdlg commented Dec 3, 2017

I just tried that:

// ~/.remarkrc
{
  "plugins": [
    "./.npm-packages/lib/node_modules/remark-preset-lint-markdown-style-guide",
    ["./.npm-packages/lib/node_modules/remark-lint-maximum-line-length", false]
  ]
}

~/.npm-packages/lib/node_modules is where my global packages are installed.

It doesn't work, and still report maximum-line-length warning.

@wooorm
Copy link
Member

wooorm commented Dec 3, 2017

Hmm. 🤔

Does ./.npm-packages/lib/node_modules/remark-lint-maximum-line-length exist?

@pvdlg
Copy link
Author

pvdlg commented Dec 3, 2017

With:

// ~/.remarkrc
{
  "plugins": [
    "~/.npm-packages/lib/node_modules/remark-preset-lint-markdown-style-guide",
    ["~/.npm-packages/lib/node_modules/remark-lint-maximum-line-length", false]
  ]
}

I get the error: Error: Could not find module ~/.npm-packages/lib/node_modules/remark-preset-lint-markdown-style-guide`
Even though:

$ ls ~/.npm-packages/lib/node_modules/remark-preset-lint-markdown-style-guide                                                                  400ms → 1 
index.js     node_modules package.json readme.md

@pvdlg
Copy link
Author

pvdlg commented Dec 3, 2017

To make it works I have to configure:

// ~/.remarkrc
{
  "plugins": [
    "./.npm-packages/lib/node_modules/remark-preset-lint-markdown-style-guide",
    ["./.npm-packages/lib/node_modules/remark-preset-lint-markdown-style-guide/node_modules/remark-lint-maximum-line-length", false]
  ]
}

@pvdlg
Copy link
Author

pvdlg commented Dec 3, 2017

Interestingly remark-lint-maximum-line-length doesn't exist anywhere other than in ./.npm-packages/lib/node_modules/remark-preset-lint-markdown-style-guide/node_modules/remark-lint-maximum-line-length

So it doesn't seems to be related to duplicate packages in the tree with different versions.

@wooorm
Copy link
Member

wooorm commented Dec 3, 2017

No, it’s exactly because global mode doesn’t dedupe!

You can use global mode, but in that case, when reconfiguring plugins, you need to point to the deeper plugin! Global mode doesn’t flatten the tree, which is why it’s not suitable for this.

npm installs stuff differently in ~/.npm-packages than in other places, so you can fix this by creating a normal node_modules somewhere, and use that for your plugins!

@pvdlg
Copy link
Author

pvdlg commented Dec 3, 2017

No, it’s exactly because global mode doesn’t dedupe!

I'm not sure it's a dedupe issue as I have only one time the package remark-lint-maximum-line-length. So when I configure ["remark-lint-maximum-line-length", false] it's necessary loaded from the same place that remark-preset-lint-markdown-style-guide loads it from.

Also does that mean to works locally you need to run npm dedupe?
Because neither npm nor yarn systematically dedupe dependencies. As you install new package duplicate will appear (this is why the npm dedupe command exists).

@wooorm
Copy link
Member

wooorm commented Dec 3, 2017

So here’s what’s going on:

  1. remark-lint-maximum-line-length is not remark-preset-lint-markdown-style-guide/node_modules/remark-lint-maximum-line-length
  2. The former is turned off, so it isn’t loaded
  3. The latter still runs

When I’m talking about deduping I mean the flatting that npm and yarn do automatically. A clean npm install creates that flattened state. Plugins load perfectly if you have that. Global installs don’t do that. You can read more about it in npm’s docs (search for --global-style).

I suggest using remark-cli globally, but don‘t store presets and plugins globally.

@pvdlg
Copy link
Author

pvdlg commented Dec 3, 2017

When I’m talking about deduping I mean the flatting that npm and yarn do automatically. A clean npm install creates that flattened state.

I understand. But as you develop and add new modules this deduped state will not be guaranteed. you would have to delete node_modules and reinstall or run npm dedupe.

I suggest using remark-cli globally, but don‘t store presets and plugins globally.

It's a working workaround indeed.

I'm not sure it's the ideal approach though. It makes it really really hard to lint markdown that is not in a node project. All other linters (eslint, stylelint) do not rely on plugin/rule identity but on names, so this problem doesn't happen. So they can be used globally with globally installed configs and plugin.

@dirkroorda
Copy link

I also have a frustratingly hard time to override a rule. I also use remark-cli globally, with a .remarkrc.yaml file in ~.
I just wiped all remark related packages from /usr/local/lib/node_modules, and inspected all remaining packages for the presence of remark related packages. No one to be found.
I am on the command-line and run remark file.md and my .remarkrc.yaml is

plugins:
  - preset-lint-recommended
  - ['lint-list-item-indent', false]
  - frontmatter
settings:
  verbose: true
  quote: "'"
  quoteSmart: true
  preferUnquoted: true
  setext: true
  closeAtx: true
  bullet: '*'
  emphasis: '*'
  spacedTable: true
  looseTable: true
  paddedTable: false
  listItemIndent: 1

My input file is

http://foo.bar/baz

* item

and this is the output.

<http://foo.bar/baz>

* item
file.md
  1:1-1:19  warning  Don’t use literal URLs without angle brackets  no-literal-urls   remark-lint
       3:3  warning  Incorrect list-item indent: add 2 spaces       list-item-indent  remark-lint

So, the rule list-item-indent is not suppressed.
If I say in the plugin section of the config:

plugins:
  - preset-lint-recommended
  - ['lint-list-item-indent', 'tab-size']
  - frontmatter

then I get an error: Error: Could not find module lint-list-item-indent
I can npm install it, but it is already inside remark-preset-lint-recommended.
If I do install it, and run the command again, lo and behold, I get the warning twice:

<http://foo.bar/baz>

* item
file.md
  1:1-1:19  warning  Don’t use literal URLs without angle brackets  no-literal-urls   remark-lint
       3:3  warning  Incorrect list-item indent: add 2 spaces       list-item-indent  remark-lint
       3:3  warning  Incorrect list-item indent: add 2 spaces       list-item-indent  remark-lint

So there is a serious problem here. I am at a loss how to get a preset to work and then changing one of its rules!

@dirkroorda
Copy link

dirkroorda commented Dec 9, 2017

There is a difference between installing rules and activating, using them.
The problem now is that a preset installs and activates a number of rules.
A later bit of configuration should be able to switch rules on and off without actually installing them.
If you enable a rule, but have not installed the rule, you should get a warning.

So my proposal would be:
Let the plugin section deal with gathering the installed rules.
Let a rule section, outside plugins, declaratively switch rules on and off. A handy option would be

extends: 
  - preset1
  - preset2

which switches on all the rules of mentioned presets, which can be individually overridden by subsequent declarations.
I think this is more like how eslint rules work.

An other thing: it would be nice if remark-stringify would look at the same options as the parser.

@OmeGak
Copy link

OmeGak commented Oct 16, 2018

Any update with this issue? I was trying configure stuff with ~/.remarkrc and install plugins globally and got affected by the same issues raised by @pvdlg and @dirkroorda.

I would suggest editing the title of this issue to make it more obvious the problem is related to globally installed plugins.

Also, I understand the proposed workaround of installing plugins locally in ~/node_modules, but is there no plan to fix this properly?

By @pvdlg:
This is why I proposed to rely on module/rule name rather than on object identity to dedupe.

@wooorm, would that be too difficult to implement in the current codebase? Would you be against PRs in that direction?

@wooorm wooorm changed the title Cannot override preset rules Multiple globally installed presets and plugins don’t work Oct 16, 2018
@wooorm
Copy link
Member

wooorm commented Oct 16, 2018

I would suggest editing the title of this issue to make it more obvious the problem is related to globally installed plugins.

Done, thanks for suggesting!

Also, I understand the proposed workaround of installing plugins locally in ~/node_modules, but is there no plan to fix this properly?

Not really, globally installed plugins is an anti-pattern, as global stuff is meant to be a top-level thing, not to be resolved and used.

This is why I proposed to rely on module/rule name rather than on object identity to dedupe.
would that be too difficult to implement in the current codebase? Would you be against PRs in that direction?

This would mean that about ±200 plugins, often from the community, would need to be updated, that we can’t rely on node’s module caching (because we load different plugins, but pick one of them because they share an identifier), and that those identifiers also need to change when a new version is published.

@wooorm wooorm added help wanted 🙏 This could use your insight or help 🐛 type/bug This is a problem 👀 no/external This makes more sense somewhere else 🙆 yes/confirmed This is confirmed and ready to be worked on 🧑 semver/major This is a change and removed 🙉 open/needs-info This needs some more info labels Aug 15, 2019
@dietergeerts
Copy link

dietergeerts commented Aug 28, 2020

This is why I proposed to rely on module/rule name rather than on object identity to dedupe.

There’s no good way of doing this, I think, but you’re the first to raise an issue about it after it existing for a while. And probably because of global installs!

Relying on module/rule name is the better solution, to not have conflicts with versions of the rule. Eslint does this too, and rules are named according to ${package}/${rule}, and it works very well.

I'm currently trying to use some presets together, and the one overwrites some rule of the other, and that's fine. It's only very strange how remark-lint is setup, as each preset needs to include remark-lint, making it too tightly coupled. A preset should define the rules, and have a peer-dependency on remark-lint, no? It will make so much more sense. Presets should also have a peer-dependency on the plugins they use. Only in this way will versions be installed once with the one defined by the consumer. It is a bit more management for the consumer, but less fragile and then you have a good overview of the things you are using.

@dietergeerts
Copy link

There is a difference between installing rules and activating, using them.

Yes, exactly, and it's better to not mix them. Presets should just turn rules on/off, and define their plugin peer-dependencies.

@dietergeerts
Copy link

dietergeerts commented Aug 28, 2020

This is why I proposed to rely on module/rule name rather than on object identity to dedupe.
would that be too difficult to implement in the current codebase? Would you be against PRs in that direction?

This would mean that about ±200 plugins, often from the community, would need to be updated, that we can’t rely on node’s module caching (because we load different plugins, but pick one of them because they share an identifier), and that those identifiers also need to change when a new version is published.

Imho, instead of doing this, it might be better to work towards a new major release where installing plugins and turning on/off rules are separated, which will make it easier to use and understand, and probably will also make it easier to implement, no? It will solve the problem mentioned in this thread, and probably a lot of other problems too. (Also, this library is not the only one that needs certain instances to be unique, which is a real pain, like mongoose etc....) I would be interested to do some work on this.

Is there any roadmap to a new major release?

@GitMurf

This comment was marked as off-topic.

@GitMurf

This comment was marked as off-topic.

@GitMurf

This comment was marked as off-topic.

@wooorm
Copy link
Member

wooorm commented Jun 23, 2023

These new comments seem off topic, as they are not about global presets/plugins, so I’ll hide them. Please do not post off-topic comments and please use the designated support channels.

If I go ahead and install the individual rule with pnpm add --save-dev remark-lint-list-item-indent then it works as expected.

Looks like you found a solution. That’s also the solution I would recommend.
I don’t understand your posts. You already have a solution.

@GitMurf
Copy link

GitMurf commented Jun 23, 2023

These new comments seem off topic, as they are not about global presets/plugins, so I’ll hide them.

@wooorm I apologize if these seemed off topic but I am referring to the original poster's initial issue "The list-item-indent rule should have been disabled, but it's still active." (see last line of initial post here #165 (comment)). I am trying to do the exact same thing where I add a preset set of rules like preset-lint-recommended and the want to disable/modify some of the individual rules that come with it like lint-list-item-indent.

The reason I posted here and why I thought it was on topic is that I was pointing out that using pnpm seemed to have a similar result/issue as the original poster's usage of global installs.

Looks like you found a solution. That’s also the solution I would recommend.
I don’t understand your posts. You already have a solution.

@wooorm I apologize if I wasn't clear. I did not view this as a solution as I believe if you individually "re"-install each of the individual rules, that has worked all along and you can modify/disable them. But the issue (in my opinion) is that these individual rules are already installed as dependencies when you install a preset like preset-lint-recommended. It becomes redundant and makes it tough if we then have to add to our package.json each individual rules coming from the preset package of rules (when they are already installed as dependencies).

My multiple comments above were iterating as I learned and found other related datapoints throughout my research. I wanted to bring full context of what I was attempting, researching and learning along the way. Does that make sense?

Essentially the TLDR of what I still consider a problem and why my proposed "workaround" of manually adding each rule to the package.json is not a solution is because these rules are already installed as dependencies and should be able to be modified / disabled without needing to also add them each to the package.json. I believe that it is because the checking for installed individual rules is assuming npm / yarn flattened node_modules where all dependencies are included in the root of the node_modules folder... but because pnpm does not flatten all dependencies into the root, it seems that remark-lint does not recognize them which my hunch is for similar reasons as the original post global install (see here #165 (comment) and here #165 (comment) and @pvdlg comment "This is why I proposed to rely on module/rule name rather than on object identity to dedupe.")

Hopefully that helps clarify. Does that make sense? Any questions? Thanks!

@wooorm
Copy link
Member

wooorm commented Jun 23, 2023

It becomes redundant and makes it tough if we then have to add to our package.json each individual rules coming from the preset package of rules (when they are already installed as dependencies).

That’s how pnpm chooses to work. If they and you want that. That’s a choice. npm doesn’t work like that. Except when installing globally (which is an anti-pattern because it doesn’t version and could easily break).

You use JSON for your config file. JSON is a shorthand for JS. Your JSON:

"plugins": [
  "preset-lint-recommended",
  [
    "remark-lint-list-item-indent",
    "space",
  ]
]

Is equivalent to the JS:

import remarkPresetLintRecommended from 'remark-preset-lint-recommended'
import remarkLintListItemIndent from 'remark-lint-list-item-indent'

plugins: [
  remarkPresetLintRecommended,
  [
    remarkLintListItemIndent,
    'space'
  ]
]

Which explains why you need to explicitly install them in pnpm. Presumably you could do: ['remark-preset-lint-recommended/node_modules/remark-lint-list-item-indent/index.js', 'space'] in pnpm too.

TLDR: we depend on how Node can resolve things. Because we run in Node. Sometimes without wrappers. So it’s simplest to work exactly like Node does always, otherwise it’d be inconsistent.

The solution to configuring many rules is: make your own preset!

@GitMurf
Copy link

GitMurf commented Jun 24, 2023

Thank you for showing me the alternative method with JS imports. That should work.

For anyone else that stumbles across a similar issue as me with pnpm, another option is to use node-linker=hoisted (https://pnpm.io/npmrc#node-linker) in your .npmrc file. It isn't a perfect solution but it flattens node_modules to operate like npm which then will work here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted 🙏 This could use your insight or help 👀 no/external This makes more sense somewhere else 🧑 semver/major This is a change 🐛 type/bug This is a problem 🙆 yes/confirmed This is confirmed and ready to be worked on
Development

No branches or pull requests

6 participants