{"action":"created","issue":{"url":"https://api.github.com/repos/nodejs/node-eps/issues/46","repository_url":"https://api.github.com/repos/nodejs/node-eps","labels_url":"https://api.github.com/repos/nodejs/node-eps/issues/46/labels{/name}","comments_url":"https://api.github.com/repos/nodejs/node-eps/issues/46/comments","events_url":"https://api.github.com/repos/nodejs/node-eps/issues/46/events","html_url":"https://github.com/nodejs/node-eps/issues/46","id":193832590,"number":46,"title":"Improving support for symlinks in the module system","user":{"login":"jasnell","id":439929,"avatar_url":"https://avatars.githubusercontent.com/u/439929?v=3","gravatar_id":"","url":"https://api.github.com/users/jasnell","html_url":"https://github.com/jasnell","followers_url":"https://api.github.com/users/jasnell/followers","following_url":"https://api.github.com/users/jasnell/following{/other_user}","gists_url":"https://api.github.com/users/jasnell/gists{/gist_id}","starred_url":"https://api.github.com/users/jasnell/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jasnell/subscriptions","organizations_url":"https://api.github.com/users/jasnell/orgs","repos_url":"https://api.github.com/users/jasnell/repos","events_url":"https://api.github.com/users/jasnell/events{/privacy}","received_events_url":"https://api.github.com/users/jasnell/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":26,"created_at":"2016-12-06T17:20:30Z","updated_at":"2016-12-07T17:28:37Z","closed_at":null,"body":"Continuation of the discussions https://github.com/nodejs/node/pull/10132 and https://github.com/nodejs/node/issues/10107 ... Please use this thread to discuss the high level architectural, merits, and long term impact of the proposed change so that the PR discussion can be focused on the actual code review."},"comment":{"url":"https://api.github.com/repos/nodejs/node-eps/issues/comments/265513479","html_url":"https://github.com/nodejs/node-eps/issues/46#issuecomment-265513479","issue_url":"https://api.github.com/repos/nodejs/node-eps/issues/46","id":265513479,"user":{"login":"isaacs","id":9287,"avatar_url":"https://avatars.githubusercontent.com/u/9287?v=3","gravatar_id":"","url":"https://api.github.com/users/isaacs","html_url":"https://github.com/isaacs","followers_url":"https://api.github.com/users/isaacs/followers","following_url":"https://api.github.com/users/isaacs/following{/other_user}","gists_url":"https://api.github.com/users/isaacs/gists{/gist_id}","starred_url":"https://api.github.com/users/isaacs/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/isaacs/subscriptions","organizations_url":"https://api.github.com/users/isaacs/orgs","repos_url":"https://api.github.com/users/isaacs/repos","events_url":"https://api.github.com/users/isaacs/events{/privacy}","received_events_url":"https://api.github.com/users/isaacs/received_events","type":"User","site_admin":false},"created_at":"2016-12-07T17:28:37Z","updated_at":"2016-12-07T17:28:37Z","body":"Here's another proposal possibility that might work.\r\n\r\n1. Track both the requested path (where the module was found) and the realpath (where the module actually is) on the module object.\r\n2. Caches remain as they currently are (using realpath as a key)\r\n3. `node_module` lookup path is the `node_module` directory set from the realpath and then the `node_module` directory set from the requested path.\r\n\r\nI haven't tested it yet, but I believe that would solve the peer dependency issue addressed by `--preserve-symlinks` (similar to how adding the cwd-based node_module lookup paths would in the git repo I linked above), and also open the door for two module-store symlink based approaches that could satisfy the necessary criteria listed above.\r\n\r\nThe first module store symlink approach is to create actual nested directory hierarchies, but symlink the contents of the packages into their destinations.  This results in a layout on disk that is more parallel to the naive \"disk is cheap\" approach, and that parallel is appealing.\r\n\r\nHowever, it necessarily means that the package manager has to keep a record of files in each package, and it's WAY more actual symbolic links.  If packages are linked from their development location _into_ the module store, then the developer will have to refresh all of their module installations whenever a file is added to the package.  (Maybe that happens rarely?  Or can be somehow detected/automated?)  So, it doesn't score very highly on the 5th heuristic (minimizing package manager complexity).\r\n\r\nThat would look something like this in practice (where `index.js` can be many files, of course)\r\n\r\n```\r\napp1\r\n├── index.js\r\n└── node_modules\r\n    ├── baz\r\n    │   ├── index.js -> ../../../.store/baz/1.2.5/index.js\r\n    │   └── node_modules\r\n    │       └── bar\r\n    │           └── index.js -> ../../../../../.store/bar/2.3.4/index.js\r\n    └── foo\r\n        ├── index.js -> ../../../.store/foo/1.2.3/index.js\r\n        └── node_modules\r\n            └── bar\r\n                └── index.js -> ../../../../../.store/bar/9.0.0/index.js\r\n```\r\n\r\nThe second module store symlink approach is to lay files out on disk with a `package.json` file containing `{\"main\":\"content\"}` and a `content` folder that links into the store.  This loses the parallelism with the naive copying approach, but it also means that package contents can be modified without a rebuild.  It also adds an additional folder and some extra file system activity on startup, so the performance impact will have to be measured.\r\n\r\nIt looks like this:\r\n\r\n```\r\napp3\r\n├── index.js\r\n└── node_modules\r\n    ├── baz\r\n    │   ├── contents -> ../../../.store/baz/1.2.5\r\n    │   ├── node_modules\r\n    │   │   └── bar\r\n    │   │       ├── contents -> ../../../../../.store/bar/2.3.4\r\n    │   │       └── package.json\r\n    │   └── package.json\r\n    └── foo\r\n        ├── contents -> ../../../.store/foo/1.2.3\r\n        ├── node_modules\r\n        │   └── bar\r\n        │       ├── contents -> ../../../../../.store/bar/9.0.0\r\n        │       └── package.json\r\n        └── package.json\r\n```\r\n\r\nI'll try to find some time to make a patch for this minimal module system change so that we can investigate it further and try to shake out some other tradeoffs.\r\n\r\nOnce there are at least 2 or 3 options, we can figure out which one satisfies the criteria the best, and with a minimum of new footguns."}}
{"actor":{"display_login":"isaacs"}}