nixfromnpm icon indicating copy to clipboard operation
nixfromnpm copied to clipboard

Dev dependencies missing from node_modules

Open paulyoung opened this issue 7 years ago • 31 comments

I generated nix expressions with the following command:

$ nixfromnpm -f ./package.json -o nixfromnpm --dev-depth 1

Despite dev dependencies appearing in ./nixfromnpm/nodePackages/default.nix and ./project.nix, they are missing from the node_modules directory when I build using the generated default.nix file.

How can I make sure they are present in node_modules?

paulyoung avatar Oct 22 '18 22:10 paulyoung

I think the answer probably lies in skipDevDependencyCleanup = true; as suggested in https://github.com/adnelson/nixfromnpm/issues/73#issuecomment-368059268, but I'm not sure where to do that just yet.

paulyoung avatar Oct 22 '18 23:10 paulyoung

I cloned the project and pointed to my local copy. After doing some sensible things to make sure dev dependency cleanup is skipped I still can't find dev dependencies in node_modules.

paulyoung avatar Oct 23 '18 00:10 paulyoung

@adnelson @ixmatus any suggestions?

paulyoung avatar Dec 07 '18 03:12 paulyoung

Are you sure they are not in the node_modules? nixfromnpm constructs a hierarchical node_modules so it could be that the dev dependencies are in the package's node_modules that depends on them. If it is at the top-level I would expect to see it at the top-level (at least we do for our usage of it with Awake's front-end javascript application).

Is this a package you can share the source to? Or at least the package.json's dependencies?

ixmatus avatar Dec 07 '18 03:12 ixmatus

My package.json looks like this:

{
  "private": true,
  "name": "my-project",
  "version": "0.1.0",
  "dependencies": {
    "borc": "^2.0.4"
  },
  "devDependencies": {
    "css-loader": "^1.0.0",
    "style-loader": "^0.23.0",
    "webpack": "^4.21.0",
    "webpack-cli": "^3.1.2",
    "webpack-dev-server": "^3.1.9"
  }
}

With the above, there's only a borc directory, no .bin directory so things like . ${nodeModules}/.bin/webpack-dev-server fail.

I'm currently doing this as a workaround:

{
  "private": true,
  "name": "my-project",
  "version": "0.1.0",
  "_dependencies": {
    "borc": "^2.0.4"
  },
  "_devDependencies": {
    "css-loader": "^1.0.0",
    "style-loader": "^0.23.0",
    "webpack": "^4.21.0",
    "webpack-cli": "^3.1.2",
    "webpack-dev-server": "^3.1.9"
  },
  "dependencies": {
    "borc": "^2.0.4",

    "css-loader": "^1.0.0",
    "style-loader": "^0.23.0",
    "webpack": "^4.21.0",
    "webpack-cli": "^3.1.2",
    "webpack-dev-server": "^3.1.9"
  }
}

paulyoung avatar Dec 07 '18 03:12 paulyoung

@paulyoung I will try to reproduce with this over the weekend.

ixmatus avatar Dec 08 '18 00:12 ixmatus

@ixmatus thanks. You might run into the same issue I did in https://github.com/adnelson/nix-node-packages/issues/40

paulyoung avatar Dec 11 '18 07:12 paulyoung

I don't usually use the nix-node-packages. I didn't have a chance over the weekend (I got sick...) but may have some time tonight or tomorrow night to try and reproduce and help out.

ixmatus avatar Dec 11 '18 14:12 ixmatus

I don't usually use the nix-node-packages.

@ixmatus what do you normally do?

I'm following the process in the README but terser-webpack-plugin (the package triggering the issue mentioned in https://github.com/adnelson/nix-node-packages/issues/40) isn't actually in the nix-node-packages

paulyoung avatar Dec 11 '18 22:12 paulyoung

@paulyoung I just don't use them. I override packages as I need to. I tried to reproduce with your package.json but ran into an issue that might be orthogonal to this one:

>>=  nix build -f ./release.nix my-project
builder for '/nix/store/9hfbgakpc8qr72dqfvmxjhdhnz43pxqq-my-project-0.1.0-nodejs-10.9.0.drv' failed with exit code 1; last 10 log lines:
  Error: This must be run in a directory with a package.json
      at Object.<anonymous> (/nix/store/xfy752w66yrlwbxin7h3nfqqsslwmjlb-node-build-tools/bin/.check-package-json-wrapped:6:9)
      at Module._compile (internal/modules/cjs/loader.js:689:30)
      at Object.Module._extensions..js (internal/modules/cjs/loader.js:700:10)
      at Module.load (internal/modules/cjs/loader.js:599:32)
      at tryModuleLoad (internal/modules/cjs/loader.js:538:12)
      at Function.Module._load (internal/modules/cjs/loader.js:530:3)
      at Function.Module.runMain (internal/modules/cjs/loader.js:742:12)
      at startup (internal/bootstrap/node.js:266:19)
      at bootstrapNodeJSCore (internal/bootstrap/node.js:596:3)
[11 built (1 failed), 22 copied (74.9 MiB), 30.5 MiB DL]
error: build of '/nix/store/9hfbgakpc8qr72dqfvmxjhdhnz43pxqq-my-project-0.1.0-nodejs-10.9.0.drv' failed

I'll be poking at this one throughout the day.

ixmatus avatar Dec 13 '18 17:12 ixmatus

@paulyoung hilariously, unrelated to your example project, I ran into an issue with terser-webpack-plugin on our own software. The issue is that it specifies an empty bin (why?) the solution I've implemented for Awake is to override the derivation is clobber the package's package.json with one in which the bin key is removed.

ixmatus avatar Dec 13 '18 22:12 ixmatus

@ixmatus any advice on how to do this is appreciated. Perhaps we could update the README?

paulyoung avatar Dec 13 '18 22:12 paulyoung

A fix (which I will put a PR out for soon) for this would be to ignore empty bin strings.

ixmatus avatar Dec 13 '18 22:12 ixmatus

@paulyoung I can provide an example here. Overriding the derivations is pretty straight-forward and follows standard Nix derivation override idioms. Examples would be useful in the README though so I will see about creating a separate PR for that.

ixmatus avatar Dec 13 '18 22:12 ixmatus

A fix (which I will put a PR out for soon) for this would be to ignore empty bin strings.

Yes, I was planning to do this but haven't looked into much. As mentioned in https://github.com/adnelson/nix-node-packages/issues/40, I think the intention is to do that already.

I still think the fix would need to be made to nix-node-packages though, right?

paulyoung avatar Dec 13 '18 22:12 paulyoung

@paulyoung here is a minimal working example of what we do, the following Nix expression is found in a file named <my-package>/default.nix and the directory <my-package> contains a directory nodePackages which was generated by nixfromnpm (i.e. nixfromnpm -f my-package.json -o my-package/nodePackages):

{ callPackage, coreutils, haskellPackages, makeWrapper,
  nodejs-10_x, pkgs, runCommand }:
let
  clobberPackageJSON = path: oldAttrs: {
    derivationOverrides.patchPhase = (oldAttrs.patchPhase or "") + ''
      cp ${path} package.json
    '';
  };
in
(import ./nodePackages {
  nodejs = nodejs-10_x;
  inherit pkgs;
}).override { overrides = nodePackagesNew: nodePackagesOld: rec {

    # We clobber package.json files for the following node packages instead of
    # using patch, because 1-line patchfiles over the entire contents is pretty
    # much the same thing (when a package.json file isn't pretty-printed). Most
    # of these hacks are to remove stuff like:
    #
    # "bin": "",
    #
    # from the toplevel.

    terser-webpack-plugin       = terser-webpack-plugin_1-1-0;
    terser-webpack-plugin_1-1-0 = nodePackagesOld.terser-webpack-plugin_1-1-0.overrideNodePackage
      (clobberPackageJSON ./terser-webpack-plugin_1-1-0.json);

  };
}

... note that we had to "clobberPackageJSON" for the following packages:

  • file-loader
  • mini-css-extract-plugin
  • url-loader
  • terser-webpack-plugin

ixmatus avatar Dec 13 '18 22:12 ixmatus

FWIW I think I'm familiar enough with overriding fields in nix attribute sets and derivations but perhaps a bit confused since I followed the README and used nix-node-packages.

paulyoung avatar Dec 13 '18 22:12 paulyoung

Super helpful, thanks!

paulyoung avatar Dec 13 '18 22:12 paulyoung

Still going to keep this issue open since it's actually about missing dev dependencies 😄

paulyoung avatar Dec 13 '18 22:12 paulyoung

And terser-webpack-plugin_1-1-0.json is the package.json for that package minus the "bin": "", line, it's the full file but the diff of the file looks like this:

--- package.json   2018-12-13 16:48:17.000000000 -0600
+++ package.json      2018-12-13 16:41:02.000000000 -0600
@@ -7,7 +7,6 @@
   "author": "webpack Contrib Team",
   "homepage": "https://github.com/webpack-contrib/terser-webpack-plugin",
   "bugs": "https://github.com/webpack-contrib/terser-webpack-plugin/issues",
-  "bin": "",
   "main": "dist/cjs.js",
   "engines": {
     "node": ">= 6.9.0 <7.0.0 || >= 8.9.0"

ixmatus avatar Dec 13 '18 22:12 ixmatus

Yes leave this open, I still have not reproduced your original issue description.

ixmatus avatar Dec 13 '18 22:12 ixmatus

@ixmatus I saw your comment about clobbering vs patching and I think I might want/need to patch anyway. How were you getting the non-pretty printed package.json file?

paulyoung avatar Dec 18 '18 15:12 paulyoung

@paulyoung I was going to the package's github project and the tagged version, then copying the package.json and pretty printing it in Emacs before writing it.

ixmatus avatar Dec 18 '18 15:12 ixmatus

I must be doing something wrong since patching that way seems to have no effect.

paulyoung avatar Dec 20 '18 11:12 paulyoung

I'm doing this:

terser-webpack-plugin_1-1-0 = nodePackagesOld.terser-webpack-plugin_1-1-0.overrideNodePackage (_: {
  derivationOverrides.apply = [
    ../patches/terser-webpack-plugin_1-1-0.patch
  ];
});

paulyoung avatar Dec 20 '18 12:12 paulyoung

Shouldn't it be:

terser-webpack-plugin_1-1-0 = nodePackagesOld.terser-webpack-plugin_1-1-0.overrideNodePackage (oldDrv: {
  derivationOverrides.patches = (oldDrv.patches or []) ++ [
    ../patches/terser-webpack-plugin_1-1-0.patch
  ];
});

ixmatus avatar Dec 20 '18 14:12 ixmatus

@ixmatus I know it's been a while but were you able to reproduce the issue?

paulyoung avatar Mar 06 '19 19:03 paulyoung

And to answer your previous questions, the docs say this:

https://github.com/adnelson/nixfromnpm/blob/aca28773b23512fded6d53a9cfdd6816387239b2/nix-libs/nodeLib/buildNodePackage.nix#L550-L559

paulyoung avatar Mar 06 '19 22:03 paulyoung

I think the issues with patch is this:

https://github.com/adnelson/nixfromnpm/blob/76775d22d743c5908b65e25574156dd624baf489/nix-libs/nodeLib/buildNodePackage.nix#L424-L437

paulyoung avatar Mar 06 '19 22:03 paulyoung

@paulyoung sorry I've been inactive on this, but how about using postPatch as in this example? https://github.com/adnelson/nix-node-packages/blob/master/nodePackages/detect-character-encoding/0.2.1.nix

Removing patchPhase like this is probably not a great idea, especially without a warning! Also it's pretty poorly documented.

adnelson avatar Mar 07 '19 00:03 adnelson