Open wojtekmaj opened 9 months ago
This is a known issue caused by the current yarn.lock
parsing mechanism, which relies on fragile assumptions.
I think I know where this error is coming from. I've added a couple of console.logs:
Diving to createPackageNode from jsx-ast-utils dependencies level:
{ depName: 'object.assign', depVersion: '^4.1.3' }
Setting to referenceMap:
object.assign { name: 'object.assign', version: '^4.1.3', dependencies: [] }
Diving to createPackageNode from top level: { depName: 'object.assign', depVersion: '1.0.21' }
referenceMap already has object.assign :
{ name: 'object.assign', version: '^4.1.3', dependencies: [] }
so will not store:
{
name: '@nolyfill/object.assign',
version: '1.0.21',
dependencies: []
}
Found 1 redundant packages:
object.assign ^4.1.3
So in my case, we're first hitting entry for jsx-ast-utils
in the lockfile:
"jsx-ast-utils@npm:^2.4.1 || ^3.0.0, jsx-ast-utils@npm:^3.3.3":
version: 3.3.3
resolution: "jsx-ast-utils@npm:3.3.3"
dependencies:
array-includes: ^3.1.5
object.assign: ^4.1.3
checksum: a2ed78cac49a0f0c4be8b1eafe3c5257a1411341d8e7f1ac740debae003de04e5f6372bfcfbd9d082e954ffd99aac85bcda85b7c6bc11609992483f4cdc0f745
languageName: node
linkType: hard
This provides information of object.assign@npm:^4.1.3 existing, but NOT about it being already nolyfilled. Then, when it gets to actual top-level object.assign entry, it skips storing it in the referenceMap because "it already has it".
Then I thought to myself, why do we even care about dependencies, if all entries of Yarn lockfile are top-level?
So I removed this bit:
and everything worked like a charm, AND a bit faster too. The final list of resolutions was identical to the one produced by the code with these lines. Still overly large (#36), but this issue was gone:
Diving to createPackageNode from top level: { depName: 'object.assign', depVersion: '1.0.21' }
Setting to referenceMap:
object.assign {
name: '@nolyfill/object.assign',
version: '1.0.21',
dependencies: []
}
Congratulations! Your project does not contain any redundant polyfills that can be optimized by nolyfill 🚀
Then I thought to myself, why do we even care about dependencies, if all entries of Yarn lockfile are top-level?
Traversing dependencies
is actually designed to solve the very problem of #36: we only need to overwrite the package as high level as possible.
So what you're saying is that my case where removal of this code did not affect the final resolutions put in the package.json is a sort of an edge case?
So what you're saying is that my case where removal of this code did not affect the final resolutions put in the package.json is a sort of an edge case?
What the nolyfill actually missing is a way to properly read and parse the yarn.lock
file. If nolyfill can parse the yarn.lock
file into a correct dependency tree, we can fix both #36 and #37 at the same time.
@SukkaW I'm willing to implement a better parser, but I will need to understand what we're missing.
@SukkaW I'm willing to implement a better parser, but I will need to understand what we're missing.
If you are interested in implementing this, here are a few things to start with:
nolyfill
CLI package is located at https://github.com/SukkaW/nolyfill/tree/master/packages/tools/clinpm.ts
, pnpm.ts
, and yarn.ts
yarn.ts
exports a method buildYarnDepTree
, which is supposed to return Promise<PackageNode[]>
PackageNode
is as follows:
export interface PackageNode {
name: string,
version: string,
dependencies?: PackageNode[]
}
yarn.lock
is parsed by @yarnpkg/parsers
(@yarnpkg/parsers/lib/syml
).