-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
jQuery and what-input should be peerDependencies #11290
Comments
Any opinion on this @DanielRuf @brettsmason ? |
What exactly is meant with this? In many cases the devDependencies are/will not installed. |
@DanielRuf When we are locally building Foundation to run tests or start the documentation (for development purposes), we need peerDependencies to be installed too. Current Yarn (and maybe NPM) versions will not always install them so we need to declare them as both |
Shouldn't be
|
I would be devDependencies if we was only delivering dist files (i.e. |
I do not mean jQuery but the dev dependencies. |
I don't understand |
|
peerDependencies is for dependencies that are exposed to (and expected to be used by) the consuming code, as opposed to "private" dependencies that are not exposed, and are only an implementation detail. * We need `jQuery` and `motion-ui` as peerDependencies instead of dependencies because we actually expect the user to install them. We cannot simply remove them because we still need the package managers to check for versions compatibilities and warn the user if they are missing. * We need `jQuery` and `motion-ui` as devDependencies too because we use them for tests/build/documentation and peerDependencies are not installed (even in development mode - yarnpkg/yarn#1503). See foundation#11290
peerDependencies is for dependencies that are exposed to (and expected to be used by) the consuming code, as opposed to "private" dependencies that are not exposed, and are only an implementation detail. We still need `jQuery` in peerDependencies instead of dependencies because we actually expect the user to install it. We cannot simply remove it because we still need the package managers to check for versions compatibilities and warn the user if it is missing. See foundation/foundation-sites#11290
@DanielRuf So, what do you think ? See #11294 |
…dencies-11290 chore: move jQuery & what-input to peerDependencies #11290
…er-dependencies-11290 for v6.5.0 b1b9408 chore: move jQuery & what-input to peerDependencies foundation#11290 Signed-off-by: Nicolas Coden <nicolas@ncoden.fr>
It made sense to do this for jQuery for all of the reasons you gave, but not for what-input. Most sites that use Foundation don't need what-input themselves, but now they all need to add it as a direct dependency of themselves anyway. |
@josephcsible You are right, Removing |
But why can't we just use our own version of |
We do not require what-input. It is perfectly safe not to install it. Since npm >= 3, npm does not install peerDependencies. This is why we also have these peerDependencies as devDependencies, to ensure we have them installed when running tests. NPM is not quite suited to the front-end environement. There is optimizations to reduce the number of installed dependencies, but it is nothing compared to the Bower non-deterministic method that forced the user to select a single version of each package. Peer dependencies are the best way to reproduce this behavior in NPM, and more than that we don't force anything to the user. No package is installed by default. I found a good resume about it in the following article: https://survivejs.com/maintenance/packaging/managing-dependencies/
Even if Foundation is not a plugin of
|
Those two sentences are my concern. I don't have any need for what-input, and neither does Foundation, but yarn complains loudly if I don't install it, since peer dependencies are considered required. |
npm throws just a warning in this case that peerDeps are missing (eg adv). Still, what-input is not required to get Foundation Sites working as it progressively enhances Foundation afaik.
Do you mean the warning? It is just a warning. |
I guess we should use https://docs.npmjs.com/files/package.json#optionaldependencies https://docs.npmjs.com/files/package.json#peerdependencies It is like a react plugin requiring react to work (peerDep). But it is not really meant as "this is not needed / required". I think package.json is often used the wrong way. Let's test with At least what-input. |
|
Hi @zkat 👋. Could you help us on this. I searched for documentation about how to deal with optional dependencies in web-oriented packages but the only thing I found is from 4 year ago (https://blog.npmjs.org/post/101775448305/npm-and-front-end-packaging). The problemFoundation is a web-oriented package, a framework that provide distribution files to import in the HTML. It relies on
The question
I feels like there is a missing concept, like
|
The developer should do this on his own / manually. The fields are documented at https://docs.npmjs.com/files/package.json. |
Created a small local test. jQuery was added as nomral dependency, what-input as optionalDependency.
|
I guess our use case is not what can be solved with standard solutions (npm, package.json). Generally other projects handle it like this: provide a But
|
@josephcsible I see no error:
|
Sure, with |
I guess either peerDeps was never meant for this (see the wording in the docs) or the wording is not really good and optionalDeps are always installed by default (which does not make them optional by default). I would say we should use |
@DanielRuf yarn gives me this when I try it:
|
It's just a warning so it is not a problem ;-) |
Warnings are totally fine in this case. |
See also: |
Closing as the initial issue is resolved. The mentioned dependencies are now peerDependencies. |
Description
I think that all current
dependencies
should be considered aspeerDependencies
for the following reasons:foudation.js
, we expect the user to importjQuery
by themself. To do this, we considerjQuery
as an external in our build process and actually reproduce thepeerDependency
behavior.jQuery
to be specific to Foundation and want it to be imported only once, like most frontend dependencies.See:
Possible Solution
Move
jquery
andwhat-input
topeerDependencies
(but alsodevDependencies
for local developement usages - see yarnpkg/yarn#1503).Checklist (all required):
The text was updated successfully, but these errors were encountered: