ilearnio / module-alias

Register aliases of directories and custom module paths in Node
MIT License
1.76k stars 69 forks source link

wrong package.json being read when used with yarn workspaces #87

Open andreialecu opened 4 years ago

andreialecu commented 4 years ago

Ran into an issue when using yarn workspaces hoisting. Because the module-alias directory will be hoisted to the top level of the project, it will read the package.json from the workspace root instead of the package's directory instead.

This will break aliases.

A workaround for this is to disable hoisting for module-alias on a package level, eg:

packages/somepkg/package.json

  "workspaces": {
    "nohoist": ["module-alias"]
  },

Then re-run yarn install --force.

I think ideally this should be handled without disabling hoisting.

andreialecu commented 4 years ago

I think it is caused by this line:

https://github.com/ilearnio/module-alias/blob/dev/index.js#L166

The order of the items in the array should be reversed.

b-jsshapiro commented 4 years ago

So first, I confirm that this is an issue. @andreialecu's workaround will work in the consuming package, but since the problem applies to any package in your tree that uses module-alias, a better workaround is to switch to the new workspaces specification syntax in the top-level package.json file. Here's a complete, if minimal file:

{
  "private": true,
  "workspaces": {
    "packages": [
      "GQLSQL",
      "SchemaTools",
      "server"
    ],
    "nohoist": ["**/module-alias"]
  }
}

With this syntax, the nohoist will get applied to all of your sub-packages, which is probably what you want. I think it would be very helpful to document this ASAP.

As to the bug itself, I don't (quite) agree with @andreialecu's proposed solution, but I do agree that the code at https://github.com/ilearnio/module-alias/blob/dev/index.js#L166 and following is not correct. If process.cwd() does contain the package.json file, and we are running the primary application (as opposed to a module), then the list constructed at L166 could very well import some completely unrelated package.json file because it checks ../.. first.

I actually think there are two bugs here. The first is that there is no way to set the base option in the package.json file -- or at least no documented way that I could find. That would let the developer say explicitly where package.json should be found.

The second bug (as I said above) is that the search strategy seems wrong. My initial thought was that we should start at either __dirname or process.cwd() (I'd have to think about which one) and walk upwards until we find a package.json, but then I realized that the package.json will always be in the directory above the node_modules directory. If this is so, then I think what we might want is:

if (options.base) {
    candidatePackagePaths = [nodePath.resolve(options.base.replace(/\/package\.json$/, ''))]
  } else {
    candidatePackagePaths = module.paths.map((s) => path.join(s, ".."));
 }

That is: look in all of the directories that are above one of the module resolution directories.

Just a thought.

Kehrlann commented 4 years ago

Hi there ! I'll try and look into it, I'm unfamiliar with yarn workspaces.

Re: the point about dependencies using module-alias . I think this module was not designed with dependencies in mind. It was explicitly stated in the README. We have now updated it with a new section explaining how to use within another NPM package . The gist of it is: avoid using require('module-alias/register'), and use either programmatic registration or require('module-alias')(__dirname). We are discussing better UX for using module-alias anywhere in #35

Re: the point about "but then I realized that the package.json will always be in the directory above the node_modules" . This does not hold if you app is deployed on a platform that creates symlinks with your node_modules , e.g. Heroku. In that case, one level above node_modules could be anywhere on your fs. See #43 for discussions.

Jeandcc commented 2 years ago

WOW, it took me forever to find this. Thank you @andreialecu for creating this issue. It's a shame that the extremely easy fix isn't widely known or shared 😢

So again, what worked for me was:

1) Add "private": true to packages/packageA/package.json. Not setting it to private prevents the nohoist config from taking place 2) Add "workspaces": {"nohoist": ["module-alias"]} to packages/packageA/package.json 3) After doing that, I deleted _nodemodules, and then ran yarn. yarn install --force should also do the trick

Frederick888 commented 3 months ago

Just bumped into this issue. Note that newer Yarn no longer has nohoist.

Set nmHoistingLimits to workspaces instead.