Add integrities to deps graph #9574
Conversation
|
the install function also return depsRequiringBuild. We need this information also when skipping install by creating the lockfile from the graph. So, maybe we should specify in the graph which dependencies require a build. |
| private _getComponentIdByPkgNameMap(): Map<string, ComponentID> { | ||
| if (!this.allWorkspaceComps) throw new Error('please make sure to populate this.allWorkspaceComps before'); | ||
| const componentIdByPkgName = new Map<string, ComponentID>(); | ||
| for (let i = 0; i < this.components.length; i++) { |
There was a problem hiding this comment.
why not using this.components.forEach?
| for (const depType of ['dependencies', 'devDependencies', 'optionalDependencies', 'specifiers', 'dependenciesMeta']) { | ||
| for (const workspacePkgName of opts.componentIdByPkgName.keys()) { | ||
| if (workspacePkgName !== opts.pkgName) { | ||
| delete lockfile.importers[importerId]?.[depType]?.[workspacePkgName] |
There was a problem hiding this comment.
@zkochan , do you have the prettier running on commit hook? I'm wondering why it didn't add the semicolon at the end of this line
There was a problem hiding this comment.
I am not sure, isn't it configured automatically?
There was a problem hiding this comment.
it should. maybe next commit, look for something running before you get the response. if it's not working, I guess full-setup should fix it.
| const pkgsToResolve: Array<{ name: string; version: string; pkgId: string }> = []; | ||
| const pkgsInTheWorkspaces = new Set(Object.values(manifests).map(({ name }) => name)); | ||
| for (const [pkgId, pkg] of Object.entries(lockfile.packages)) { | ||
| if (pkg.resolution == null || 'type' in pkg.resolution && pkg.resolution.type === 'directory') { |
There was a problem hiding this comment.
you can probably replace 'type' in pkg.resolution && pkg.resolution.type === 'directory' with pkg.resolution?.type === 'directory'
There was a problem hiding this comment.
I can't because it is a union type and not every type has the type field.
e2e/harmony/deps-graph.e2e.ts
Outdated
| helper.command.setConfig('registry', npmCiRegistry.getRegistryUrl()); | ||
|
|
||
| helper.fixtures.populateComponents(2); | ||
| helper.extensions.workspaceJsonc.addKeyValToDependencyResolver('rootComponents', true); |
There was a problem hiding this comment.
isn't this one comes already by default? the rootComponents true?
e2e/harmony/deps-graph.e2e.ts
Outdated
| helper.command.export(); | ||
| helper.command.sign( | ||
| [`${helper.scopes.remote}/[email protected]`], | ||
| '--log', |
e2e/harmony/deps-graph.e2e.ts
Outdated
| helper.scopeHelper.reInitLocalScope(); | ||
| helper.scopeHelper.addRemoteScope(); | ||
| helper.command.import(`${helper.scopes.remote}/comp1@latest`); | ||
| expect(fs.readFileSync(path.join(helper.scopes.localPath, 'pnpm-lock.yaml'), 'utf8')).to.have.string( |
There was a problem hiding this comment.
you can replace fs.readFileSync(path.join(helper.scopes.localPath, 'pnpm-lock.yaml'), 'utf8') by helper.fs.readFile('pnpm-lock.yaml')
Proposed Changes