Skip to content
This repository has been archived by the owner on Jan 20, 2022. It is now read-only.

[BUG] bad behavior in npm/cli#750 test case #43

Closed
isaacs opened this issue Feb 4, 2020 · 1 comment · Fixed by #44
Closed

[BUG] bad behavior in npm/cli#750 test case #43

isaacs opened this issue Feb 4, 2020 · 1 comment · Fixed by #44
Assignees

Comments

@isaacs
Copy link
Contributor

isaacs commented Feb 4, 2020

Set up a folder like described in npm/cli#750

Run the scripts/reify.js script on it, with --save

  1. Does not create link to ./lib in node_modules.
  2. symlink is absolute url, should be relative.
  3. package-lock.json looks like this:
{
  "name": "monorepo",
  "lockfileVersion": 2,
  "requires": true,
  "packages": {
    "": {
      "name": "monorepo",
      "dependencies": {
        "app": "file:./app"
      }
    },
    "app": {},
    "node_modules/app": {
      "resolved": "app",
      "link": true
    }
  },
  "dependencies": {
    "app": {
      "version": "file:app"
    }
  }
}

Run scripts/reify.js --save on it again, and now package-lock.json looks like this:

{
  "name": "monorepo",
  "lockfileVersion": 2,
  "requires": true,
  "packages": {
    "": {
      "name": "monorepo",
      "dependencies": {
        "app": "file:./app"
      }
    },
    "app": {
      "resolved": "git+ssh://[email protected]/node_modules/app.git"
    },
    "node_modules/app": {
      "resolved": "app",
      "link": true
    }
  },
  "dependencies": {
    "app": {
      "version": "file:app"
    }
  }
}

Super weird, looks like it's interpreting the node_modules/app reference as a github shorthand? That's wrong.

@isaacs isaacs self-assigned this Feb 4, 2020
@isaacs
Copy link
Contributor Author

isaacs commented Feb 4, 2020

We should do deep dep resolution of symlink targets when the realpath is under the same root folder.

Not clear whether it's better to have it in the root node_modules folder or if app/node_modules/lib should be a symlink to the lib folder. I think that's probably better.

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

Successfully merging a pull request may close this issue.

1 participant