Conversation
Co-Authored-By: Maël Nison <nison.mael@gmail.com>
arcanis
left a comment
arcanis
left a comment
There was a problem hiding this comment.
This seems good to me - I left a few comments here and there to be sure we don't lose some context over the time, but the code is fairly understandable if we spend the time so I'm not too worried.
I'll rebase and merge, followups can be made in other PRs. Thanks for your incredible work!
| private persistPath(dir: PortablePath) { | ||
| const pathStack = []; | ||
| let curPath = dir; | ||
| while (!this.baseFs.existsSync(curPath)) { | ||
| pathStack.push(curPath); | ||
| curPath = ppath.dirname(curPath); | ||
| } | ||
| for (const fullPath of pathStack.reverse()) { | ||
| this.baseFs.mkdirSync(fullPath); | ||
| } | ||
| } |
There was a problem hiding this comment.
Would mkdir with recursive: true work?
| try { | ||
| fsDirList = await this.baseFs.readdirPromise(pnpPath.resolvedPath); | ||
| } catch (e) { | ||
| // Ignore errors | ||
| } |
There was a problem hiding this comment.
Mentioned on Discord: some packages (like fsevents) seem to generate their own physical node_modules folders somehow, which then need to be concatenated with the virtual content.
| * /home/user/project/node_modules/foo -> {target: '/home/user/project/.yarn/.cache/foo.zip/node_modules/foo', linkType: 'HARD'} | ||
| * /home/user/project/node_modules/bar -> {target: '/home/user/project/packages/bar', linkType: 'SOFT'} | ||
| */ | ||
| export type NodeModulesTree = Map<PortablePath, {dirList: Set<Filename>} | {dirList?: undefined, target: PortablePath, linkType: LinkType}>; |
|
|
||
| for (const [name, reference] of pkg.packageDependencies) { | ||
| if (reference !== null) { | ||
| const locator = typeof reference === 'string' ? {name, reference} : {name: reference[0], reference: reference[1]}; |
There was a problem hiding this comment.
| const locator = typeof reference === 'string' ? {name, reference} : {name: reference[0], reference: reference[1]}; | |
| const locator = pnp.getLocator(name, reference); |
(new utility)
| const allDepIds = new Set([...deps, ...peerDeps]); | ||
| for (const depId of allDepIds) { | ||
| const depPkg = packageInfos[depId]; | ||
| if (depPkg) { |
There was a problem hiding this comment.
When is it false? Is it just for TS to be happy (if so I'd prefer an assertion)?
2 participants
What's the problem this PR addresses?
This is an ongoing work to add hoisting support to NodeModulesFS and
pnpify. The PR is just created as a heads up, the development is on early stages now.How did you fix it?
At the moment draft abstract hoister has been implemented to the degree that some unit tests pass (the code quality and docs are still TBD). It is a divide and conquer algorithm with weights earlier discussed on discord.