Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support pnpm module layout #24

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .npmrc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
enable-pre-post-scripts = true
18 changes: 17 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,4 +40,20 @@ Returns `Promise<Module[]>`

Will walk your entire node_modules tree reporting back an array of "modules", each
module has a "path", "name" and "depType". See the typescript definition file
for more information.
MichaelBelousov marked this conversation as resolved.
Show resolved Hide resolved
for more information.

## Testing with pnpm

To test with pnpm, switch the definition in `package.json#scripts#pretest` to `npm run pretest:pnpm`. instead of `npm run pretest:yarn`.

You may notice that we use a particular version of pnpm when testing, this is because
it may complain about the lockfile not being in sync with the package.json and fail.
This seems to be a bug in pnpm because it won't use the different but compatible lockfile when
`--frozen-lockfile` is specified.

For now, you can just force a particular pnpm version to run the tests like we did.

```sh
pnpx pnpm@~6.1 install
pnpx pnpm@~6.1 run test
```
4 changes: 3 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@
"scripts": {
"build": "tsc",
"prepare": "npm run build",
"pretest": "cd test/fixtures/xml2js && yarn --frozen-lockfile",
"pretest": "npm run pretest:yarn",
"pretest:yarn": "cd test/fixtures/xml2js && yarn --frozen-lockfile",
"pretest:pnpm": "cd test/fixtures/xml2js && pnpx -y pnpm@~6.1 install --frozen-lockfile",
"test": "mocha --require ts-node/register test/*_spec.ts"
},
"dependencies": {
Expand Down
61 changes: 34 additions & 27 deletions src/Walker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export interface Module {
depType: DepType;
nativeModuleType: NativeModuleType,
name: string;
depth: number;
}

const d = debug('flora-colossus');
Expand All @@ -34,10 +35,6 @@ export class Walker {
this.rootModule = modulePath;
}

private relativeModule(rootPath: string, moduleName: string) {
return path.resolve(rootPath, 'node_modules', moduleName);
}

private async loadPackageJSON(modulePath: string): Promise<PackageJSON | null> {
const pJPath = path.resolve(modulePath, 'package.json');
if (await fs.pathExists(pJPath)) {
Expand All @@ -50,33 +47,34 @@ export class Walker {
return null;
}

private async walkDependenciesForModuleInModule(moduleName: string, modulePath: string, depType: DepType) {
let testPath = modulePath;
let discoveredPath: string | null = null;
let lastRelative: string | null = null;
// Try find it while searching recursively up the tree
while (!discoveredPath && this.relativeModule(testPath, moduleName) !== lastRelative) {
lastRelative = this.relativeModule(testPath, moduleName);
if (await fs.pathExists(lastRelative)) {
discoveredPath = lastRelative;
} else {
if (path.basename(path.dirname(testPath)) !== 'node_modules') {
testPath = path.dirname(testPath);
private async walkDependenciesForModuleInModule(moduleName: string, modulePath: string, depType: DepType, depth: number) {
let discoveredPath: string | undefined;
try {
// Use the require machinery to resolve the package.json of the given module.
discoveredPath = path.dirname(require.resolve(`${moduleName}/package.json`, { paths: [modulePath] }));
} catch (err) {
if (err.code === 'ERR_PACKAGE_PATH_NOT_EXPORTED') {
// package did not export package.json so we're going to steal the path from the error message as a fallback
// yes we're relying on this, we should instead try to directly rely on node's resolution algorithm, which means
// finding somewhere where it is exposed (I can't find it), or copying it
discoveredPath = path.dirname(err.message.match(/in (?<path>.+)$/).groups.path);
if (!await fs.pathExists(discoveredPath)) {
throw new Error('error did not end in an "in" clause with a path');
}
testPath = path.dirname(path.dirname(testPath));
}
}
// If we can't find it the install is probably buggered
if (!discoveredPath && depType !== DepType.OPTIONAL && depType !== DepType.DEV_OPTIONAL) {
throw new Error(
`Failed to locate module "${moduleName}" from "${modulePath}"
} finally {
// If we can't find it the install is probably buggered
if (!discoveredPath && depType !== DepType.OPTIONAL && depType !== DepType.DEV_OPTIONAL) {
throw new Error(
`Failed to locate module "${moduleName}" from "${modulePath}"

This normally means that either you have deleted this package already somehow (check your ignore settings if using electron-packager). Or your module installation failed.`
);
This normally means that either you have deleted this package already somehow (check your ignore settings if using electron-packager). Or your module installation failed.`
);
}
}
// If we can find it let's do the same thing for that module
if (discoveredPath) {
await this.walkDependenciesForModule(discoveredPath, depType);
await this.walkDependenciesForModule(discoveredPath, depType, depth + 1);
}
}

Expand All @@ -89,7 +87,7 @@ export class Walker {
return NativeModuleType.NONE
}

private async walkDependenciesForModule(modulePath: string, depType: DepType) {
private async walkDependenciesForModule(modulePath: string, depType: DepType, depth: number) {
d('walk reached:', modulePath, ' Type is:', DepType[depType]);
// We have already traversed this module
if (this.walkHistory.has(modulePath)) {
Expand All @@ -98,10 +96,15 @@ export class Walker {
const existingModule = this.modules.find(module => module.path === modulePath) as Module;
// If the depType we are traversing with now is higher than the
// last traversal then update it (prod superseeds dev for instance)
// NOTE: doesn't this cause the pruning logic to to not prune dev>prod dependencies?
if (depTypeGreater(depType, existingModule.depType)) {
d(`existing module has a type of "${existingModule.depType}", new module type would be "${depType}" therefore updating`);
existingModule.depType = depType;
}
// If the depth we are traversing is less, update it
if (depth < existingModule.depth) {
existingModule.depth = depth;
}
return;
}

Expand All @@ -120,6 +123,7 @@ export class Walker {
nativeModuleType: await this.detectNativeModuleType(modulePath, pJ),
path: modulePath,
name: pJ.name,
depth
});

// For every prod dep
Expand All @@ -134,6 +138,7 @@ export class Walker {
moduleName,
modulePath,
childDepType(depType, DepType.PROD),
depth
);
}

Expand All @@ -143,6 +148,7 @@ export class Walker {
moduleName,
modulePath,
childDepType(depType, DepType.OPTIONAL),
depth
);
}

Expand All @@ -154,6 +160,7 @@ export class Walker {
moduleName,
modulePath,
childDepType(depType, DepType.DEV),
depth
);
}
}
Expand All @@ -166,7 +173,7 @@ export class Walker {
this.cache = new Promise<Module[]>(async (resolve, reject) => {
this.modules = [];
try {
await this.walkDependenciesForModule(this.rootModule, DepType.ROOT);
await this.walkDependenciesForModule(this.rootModule, DepType.ROOT, 0);
} catch (err) {
reject(err);
return;
Expand Down
10 changes: 4 additions & 6 deletions test/Walker_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,6 @@ describe('Walker', () => {
});

describe('conflicting optional and dev dependencies (xml2js)', () => {
const deepIdentifier = path.join('xml2js', 'node_modules', 'plist');

beforeEach(async () => {
modules = await buildWalker(path.join(__dirname, 'fixtures', 'xml2js'));
});
Expand All @@ -82,10 +80,10 @@ describe('Walker', () => {

it('should detect the hoisted and unhoisted instances correctly as optional/dev', () => {
const xmlBuilderModules = modules.filter(m => m.name === 'xmlbuilder');
// Kept deep by plist
const expectedDev = xmlBuilderModules.find(m => m.path.includes(deepIdentifier));
// Hoisted for xml2js
const expectedOptional = xmlBuilderModules.find(m => !m.path.includes(deepIdentifier));
// versions tested come from the lockfile, 8.2.2 is the version depended upon by plist@2.x
const expectedDev = xmlBuilderModules.find(m => m.path.includes("8.2.2"));
// versions tested come from the lockfile, 9.0.7 is the version transitively depended upon by xml2js and [email protected]
const expectedOptional = xmlBuilderModules.find(m => m.path.includes("9.0.7"));
expect(expectedDev).to.have.property('depType', DepType.DEV);
expect(expectedOptional).to.have.property('depType', DepType.OPTIONAL);
});
Expand Down
7 changes: 7 additions & 0 deletions test/fixtures/xml2js/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,12 @@
"galactus": "^0.2.1",
"jimp": "^0.2.27",
"plist": "^2.0.1"
},
"//": [
"the following resolutions below force pnpm and yarn to not install a later version of a dependency",
"with a new amount of duplicates of a required module"
],
"resolutions": {
"xml2js": "0.4.19"
}
}
Loading