[RRFC] Platform-Specific Dependencies #120
Replies: 4 comments 3 replies
-
I think the problem identified here is a great one to discuss, I agree it is a problem we should try to solve. That said I am concerned the solution is very complicated when really what we want is not to show this kind of warning to the user at install time. Seems like we should also consider other solutions before considering this rather complicated one. |
Beta Was this translation helpful? Give feedback.
-
There are two changes coming in npm v7 that make this less of a problem, and may make this feature less necessary. The first is that build failures for optional dependencies are not displayed to the user. Install scripts are run in a child process and their output collected, and then only printed if the script exits with a non-zero error code. Second, optional dependencies that specify an engine/os/cpu configuration that is not met will be ignored when building the tree. (Not fetched, not unpacked, etc.) So, The tricky thing about platform-specific dependencies is that we may end up constructing a radically different tree on different platforms. Ie, not just "fsevents and its deps are not installed", but "linux gets foo@1 and darwin gets foo@2, in the same place in the tree". That makes it much harder to have build consistency, and leads to a lot of "works on my machine". (To some extent, optionalDependencies with platform declarations can lead to this as well, but it's a smaller surface.) The other challenging thing about going down this road is that creating a declarative syntax for case statements inevitably leads to surprising amounts of complexity. (#129 is a great example of this!) Especially when we add negating, there's lots of opportunity for contradictions and confusing edge cases. That's not to say we can't or shouldn't do something like this, just that there's more design cost than it might appear at first. Before ratifying or implementing this feature, we'd have to do quite a bit of work to explore those edge cases as best as possible, or end up regretting it for years to come. |
Beta Was this translation helpful? Give feedback.
-
Coming to this RFC after investigating options for an existing |
Beta Was this translation helpful? Give feedback.
-
Another solution that could be implemented at the package manager level is expanding binary targets to include platform discriminators - allowing them to work in combination with Will also need some mechanism to forward the {
"name": "@alshdavid/mach",
"version": "0.0.25",
"license": "MIT",
"bin": {
"mach": "bin/mach"
},
"binMeta": {
"mach": [
{ "os": ["win32"], "arch": ["x64"], "forward": "@alshdavid/mach-windows-amd64#bin#mach" },
{ "os": ["linux"], "arch": ["x64"], "forward": "@alshdavid/mach-linux-amd64#bin#mach" },
{ "os": ["darwin"], "arch": ["x64"], "forward": "@alshdavid/mach-macos-amd64#bin#mach" },
// etc
]
},
"optionalDependencies": {
"@alshdavid/mach-linux-amd64": "0.0.25",
"@alshdavid/mach-linux-arm64": "0.0.25",
"@alshdavid/mach-macos-amd64": "0.0.25",
"@alshdavid/mach-macos-arm64": "0.0.25",
"@alshdavid/mach-windows-amd64": "0.0.25",
"@alshdavid/mach-windows-arm64": "0.0.25"
}
}
|
Beta Was this translation helpful? Give feedback.
-
Motivation ("The Why")
Everybody has seen those warnings from an npm install command where it didn't install an optional dependency, probably because the dependency in question was OS-specific to an OS the installer wasn't running on. A common issue is a warning that fsevents wasn't installed on an OS that wasn't OSX. Furthermore, there are hundreds of packages that are Windows-specific.
Right now, these print as console warnings during the installation process. This shouldn't be a warning, because npm is behaving the way it's supposed to and the correct packages are being installed.
Packages should be allowed to specify that a particular dependency is only used on certain platforms and not others.
Furthermore, the current system can obscure problems. if fsevents is required on OSX in a package and not elsewhere, and it fails to install due to something going wrong on an OSX client, the OSX user and logs will still see that it was just an optional dependency failing to install, even though it was really a mandatory dependency on that platform.
Right now, package.json has devDependencies, dependencies, peerDependencies, bundledDependencies, and optionalDependencies as dependency types; and packages can specify the intended architectures and operating systems of the packages themselves. To fix this problem of warnings that aren't warning about something wrong, the dependency set should be extended to include "platformDependencies".
Example
Right now, chokidar specifies fsevents as an optional depdendency. This means that whenever chokidar is installed on something other than OSX, there is a warning printed. Allowing platform-specific dependencies would solve this issue.
As god-king-emperor StackOverflow has shown, this is a problem in practice: https://stackoverflow.com/questions/38021422/nodejs-npm-install-platform-specific-packages
How
Current Behaviour
Currently, platform-specific dependencies are just treated as optionalDependencies, as shown here: https://github.com/paulmillr/chokidar/blob/e1c37c6207b0b9b970f6479a9a7d238bd3b4de92/package.json
Desired Behaviour
There are several ways this could be done. I think the least awful way to do this is the following:
There would be a set of objects where a set of OSes and architectures can be specified along with the actual packages. At least 1 of the 2 of os and arch would be required but both being usable, each consisting of at least 1 target. As can be done in the current os and cpu values, the not! syntax can be used on platforms where not required.
References
Note: Currently, npm allows packages to specify that they're specific to an OS or architecture. This is a good feature that should be retained. This RFC is not meant to replace this feature but be used alongside it. optionalDependencies is also not being proposed for deprecation because it has use-cases beyond this poorly-fitting one.
Beta Was this translation helpful? Give feedback.
All reactions