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

Fix Poetic not working with NPM #28

Open
arianacosta opened this issue Nov 9, 2019 · 7 comments
Open

Fix Poetic not working with NPM #28

arianacosta opened this issue Nov 9, 2019 · 7 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@arianacosta
Copy link
Owner

Poetic is not working when installing it with NPM. It seems that the nested node_modules folder structure that NPM creates does not play well with it.

@arianacosta arianacosta added the bug Something isn't working label Nov 9, 2019
@arianacosta
Copy link
Owner Author

Confirmed that this is the case. Unlike Yarn, NPM nests the dependencies, so it needs to use peerDependencies, but there are several problems with this:

  1. NPM version 3 does not install peerDependencies automatically (possible solution https://www.npmjs.com/package/install-peerdeps)
  2. Maintaining dependencies that are peer dependencies requires manually updating the version to upgrade
  3. Having multiple dependencies goes against the idea of having a single dependency for all the linting needs

At this time we need to evaluate if we really need to support NPM, as it seems that it has more problems than the ones it solves.

@ktiedt
Copy link

ktiedt commented Nov 11, 2019

@arianacosta I'd love to help expedite this, with a bit of info to better understand your concerns I would have some idea of the best path to take, so some questions related to your points above.

  1. Why are we concerned with a 4 year old NPM version? It is no longer even supported by NPM from my understanding (outside of security updates maybe?) This seems counterproductive.
  2. I'm not sure how peer dependencies comes into play here, neither yarn or npm are technically listed as dependencies for nodejs packages (more specifically, here)
  3. The only added dependency here would be a package like https://www.npmjs.com/package/yarn-or-npm that lets you write utility scripts to support either path...

Lastly, I don't understand the concern about nested node_modules structure vs flat, linting and code formatting should only touch project owned files, not dependencies in the node_modules folder...

Is there something I am missing here?

@arianacosta
Copy link
Owner Author

arianacosta commented Nov 12, 2019

Hi @ktiedt, Thanks, I'm looking forward to having the option to NPM as well. Here are the limitations I've found so far. As soon as we can find elegant solutions to this we can implement it. To expand on the points above:

First problem: In order for ESLint to be recognized by the IDE it needs to be installed directly in node_modules/eslint or globally (https://eslint.org/docs/user-guide/getting-started#installation-and-usage). When using yarn this is the case because it doesn't nest dependencies, but NPM will create node_modues/poetic/node_modules/eslint. NPM approach to this is to use peerDependencies.

1 and 2. The problem of peerDependencies not being installed by default started with NPM version 3 all the way to the current version. You can only declare the peerDependencies in poetic, but also they need to be set in the host project's package.json as devDependencies. The thing is that Poetic will no longer update eslint automatically.

Example: Months after installing Poetic, there's a new version of ESLint and Poetic upgrades it's peerDependencies. This will only produce a warning (or error) of dependency version mismatch, but the person would need to upgrade the eslint version manually.

3. This is a great find! We can definitely use it.

Second problem:

Even if we manage to install eslint and prettier they both require other peerDependencies. For instance, when using npm run code:lint it will throw an error.

Oops! Something went wrong! :(
ESLint: 6.6.0.
ESLint couldn't find the plugin "@typescript-eslint/eslint-plugin".

To solve this, we would also need to move these dependencies to the host project as devDependencies. In my opinion, that defeats the purpose of what Poetic is trying to solve.

Maybe there's a workaround to these limitations, and I'm happy to look into them. I'll keep this ticket open in case someone has a solution.

Thanks!

@arianacosta arianacosta added enhancement New feature or request help wanted Extra attention is needed and removed bug Something isn't working labels Nov 12, 2019
@ktiedt
Copy link

ktiedt commented Nov 12, 2019

@arianacosta ahhh that makes sense, I was looking at it from the wrong package issue. One solution may be to simply include instructions on how to configure the eslint plugin if it fails to detect eslint. This of course doesn't solve the update issue, meaning poetic would basically have to update everytime eslint or prettier release.... Of course, there is also no harm in saying "this project requires you to install eslint and prettier by running the following command: npm i -D [packages] (and or yarn equiv) and let poetic manage the rest... it would still be an improvement over 100% self setup.

eslint.nodePath - use this setting if an installed ESLint package can't be detected, for example /myGlobalNodePackages/node_modules.

https://github.com/microsoft/vscode-eslint

@arianacosta
Copy link
Owner Author

@ktiedt Yeah, we could do that. I'll prepare a proof of concept to see what would be the best approach for this. Feel free to share any findings. Thank you very much for helping out!

@arianacosta arianacosta mentioned this issue Nov 26, 2019
4 tasks
@oleg-koval
Copy link
Collaborator

Is there still a problem with install?

@arianacosta
Copy link
Owner Author

@oleg-koval With Yarn everything is working as expected, but we haven't been able to make it work with NPM.

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

No branches or pull requests

3 participants