hensm / fx_cast

Chromecast Web Sender SDK implementation for Firefox
https://hensm.github.io/fx_cast/
MIT License
1.87k stars 63 forks source link

Undeclared fs-extra dependency in app #178

Closed kevincox closed 3 years ago

kevincox commented 3 years ago

Description https://github.com/hensm/fx_cast/blob/c45a45616abfda85b4e404dbf059fb11a3914a2c/app/package.json doesn't declare fs-extra as a dependency but it is used https://github.com/hensm/fx_cast/blob/c45a45616abfda85b4e404dbf059fb11a3914a2c/app/bin/build.js#L3.

Steps to reproduce

  1. Have no node packages installed.
  2. cd app
  3. npm build

Expected behaviour It builds.

Logs

> @ build /build/source/app
> node bin/build.js

internal/modules/cjs/loader.js:883
  throw err;
  ^

Error: Cannot find module 'fs-extra'
Require stack:
- /build/source/app/bin/build.js
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:880:15)
    at Function.Module._load (internal/modules/cjs/loader.js:725:27)
    at Module.require (internal/modules/cjs/loader.js:952:19)
    at require (internal/modules/cjs/helpers.js:88:18)
    at Object.<anonymous> (/build/source/app/bin/build.js:3:12)
    at Module._compile (internal/modules/cjs/loader.js:1063:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1092:10)
    at Module.load (internal/modules/cjs/loader.js:928:32)
    at Function.Module._load (internal/modules/cjs/loader.js:769:14)
    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:72:12) {
  code: 'MODULE_NOT_FOUND',
  requireStack: [ '/build/source/app/bin/build.js' ]
}
hensm commented 3 years ago

Hi, build instructions are here.

Some dependencies are shared between the extension and the app in the root directory: https://github.com/hensm/fx_cast/blob/c45a45616abfda85b4e404dbf059fb11a3914a2c/package.json#L32

kevincox commented 3 years ago

I'm trying to package it for offline building and it makes it a bit difficult as I basically need to merge the two sets of dependencies. I'm also surprised that you haven't run into versioning issues with two separate modules. I guess I'll try to merge the both of them but this is an unusual approach as far as I am aware.

hensm commented 3 years ago

Yeah, it's not ideal. If you have any suggestions, I'd be happy to make changes to make this more straightforward.

kevincox commented 3 years ago

I'm not really sure. I would just think to specify the requirements in the subdirectories instead of having any in the root. It may cause a bit of duplication but that is probably not an issue can be advantageous if you have a reason for the difference between the different components.

I did mange to get it working. For context I was trying to update the Nix package for the bridge: https://github.com/NixOS/nixpkgs/pull/121963. It is a bit weird with the dual package lists but it isn't unmanageable.

hensm commented 3 years ago

Okay, I think what I'll do is just put all the app dependencies in the app subdirectory so that can be built fully self-contained and keep a few shared dependencies in root for some of the other stuff like linting/testing.

kevincox commented 3 years ago

Thanks! I've updated the PR and it looks much cleaner.