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

Cannot find package.json in path #81

Closed
elfenheart opened this issue Oct 31, 2019 · 5 comments · Fixed by #83
Closed

Cannot find package.json in path #81

elfenheart opened this issue Oct 31, 2019 · 5 comments · Fixed by #83

Comments

@elfenheart
Copy link

Hi,

I'm using module-alias with ncc compiler (https://github.com/zeit/ncc) and seeing weird behavior. If I run the code without the ncc compiler, then it's working fine. However, after ncc compiler, then module-alias is throwing error that it cannot find package.json at some path, even though the path does contain package.json file.

I tried with passing a static path for the module-alias init, and still getting the same error.

After debugging and trying to modify the code of module-alias, I find that if I were to add a temporary variable to join the path first, before the npmPackage require being called, then things will work.

line 171

for (var i in candidatePackagePaths) {
    try {
      base = candidatePackagePaths[i]

      var packageJsonPath = nodePath.join(base, 'package.json')
      npmPackage = require(packageJsonPath)
      break
    } catch (e) {
      // noop
    }
  }

I'm not really sure why assigning the path to a temporary variable before require fixes the problem. Any idea?

@Kehrlann
Copy link
Collaborator

Kehrlann commented Oct 31, 2019

Hello !

Thanks for reporting. I don't know the NCC compiler, but I'll take a look. My first guess is from the docs:

Caveats
Files / assets are relocated based on a static evaluator. Dynamic non-statically analyzable asset loads may not work out correctly

So maybe require(nodePath.join(base, 'package.json')) throws off the static evaluator, while having it split, just like your version does, allows detection to work properly.

I'll try to make time and dig into it, I don't know exactly when, but I'll let you know if I find something.

@elfenheart
Copy link
Author

Thanks @Kehrlann!

I can submit a PR to add this change, but holding off until we understand this problem better... it's too weird

@Kehrlann
Copy link
Collaborator

Kehrlann commented Nov 8, 2019

Hello @elfenheart ,

tl;dr: we can't help from module-alias, this is a PR for NCC instead ... and even with that, the user experience wouldn't be great. My recommandation is to avoid module-alias when using NCC, sorry 😞

I've taken a look at this issue. Here are my findings. Basically NCC follows the require('module-alias') by copying the whole code of module-alias inside the output file. Now I've looked at the problematic piece of code you've pointed at, and tried with and without inlining that variable.

NCC-generated code without changing module alias

If you keep it inline, you have the following generated code by NCC:

base = candidatePackagePaths[i]

npmPackage = __ncc_wildcard$0(base)

Which in turns points to:

function __ncc_wildcard$0 (arg) {
  if (arg === "/") return __webpack_require__(727);
}

and 727 is a description of module-alias as imported ; but anyway, in this case, it returns undefined instead of throwing ; and therefore it does not look for other possible locations of package.json. After, we check that npmPackage is truthy, hence the error.

NCC-generated code with your proposed change

If you extract it, you have:

var x = nodePath.join(base, 'package.json')
npmPackage = require(x)

Which is nice, makes it import package.json and makes it not blow up. However !

Why the proposed change is not enough

  1. That package.json is not included in your bundle, and you would need to ship it alongside your dist/index.js
  2. The code of the aliased modules are not embedded, so your aliasing does not work.

To verify:

  1. Try getting this project: https://github.com/Kehrlann/module-alias-aws-lambda
  2. Then run node local.js, you get a console message:
tracing ...
hello from some deep module
  1. Then run ncc build local.js -o dist && node dist/index.js, you should be the same message however you get:
tracing ...
undefined

Because the alias failed and you get an empty module instead of the aliased module. The code of the aliased module is not included in dist/index.js, so this will never work.

So my guess is:

  1. NCC uses webpack under the hood.
  2. Webpack reads your source files to compile (bundle) them.
  3. In that process, Webpack reads the require statements, without actually calling the node's require.
  4. From those statements, Webpack finds the required modules with it's own heuristics, and "statically" links the required modules by copy-pasting them in your destination bundle.
  5. However, since it's doing this without using node's require and module-alias only adds more functionality on tope of node's require, then webpack does not understand the aliases and just gives you empty modules instead - I don't fully understand why.

So that means we can't really help from module-alias. However, there's hope !

What to do next

So basically NCC needs to use Webpack's aliasing when building, and you need those aliases to match those declared for module-alias.

So that would be a change in NCC, because for now they don't let you use aliases (see their webpack config: https://github.com/zeit/ncc/blob/0ad6971d202ee5896310f65e9aedfd1156184346/src/index.js#L112,L259)

I'm keeping this open, maybe I'll update the docs to mention that module-alias does not work with NCC.

@Kehrlann Kehrlann closed this as completed Nov 8, 2019
@Kehrlann Kehrlann reopened this Nov 8, 2019
@Kehrlann
Copy link
Collaborator

I just looked up the issues and PRs in NCC, and it seems they don't want to expose Webpack internals, including resolve.alias - see this rejected PR: vercel/ncc#460

@elfenheart
Copy link
Author

Hi @Kehrlann,

Thank you so much for the detailed analysis and explanation!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants