Package still detected as redundant *after* it has been "nolyfilled"

[wojtekmaj]

[SukkaW]

This is a known issue caused by the current yarn.lock parsing mechanism, which relies on fragile assumptions.

[wojtekmaj]

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:

nolyfill/packages/tools/cli/src/lockfile/yarn.ts

Lines 52 to 54 in 2106f9b

	packageNode.dependencies = Object.entries(yarnPkg?.dependencies || {}).map(([depName, depVersion]) => {
	return createPackageNode(depName, depVersion);
	});

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 🚀

[SukkaW]

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.

[wojtekmaj]

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?

[SukkaW]

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.

[wojtekmaj]

@SukkaW I'm willing to implement a better parser, but I will need to understand what we're missing.

[SukkaW]

@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:

• The nolyfill CLI package is located at https://github.com/SukkaW/nolyfill/tree/master/packages/tools/cli
• The lockfile parsing parts are located at https://github.com/SukkaW/nolyfill/tree/master/packages/tools/cli/src/lockfile
  • In the directory, there are npm.ts, pnpm.ts, and yarn.ts
• yarn.ts exports a method buildYarnDepTree, which is supposed to return Promise<PackageNode[]>
  • The type of PackageNode is as follows:

    export interface PackageNode {
      name: string,
      version: string,
      dependencies?: PackageNode[]
    }

• Currently, the yarn.lock is parsed by @yarnpkg/parsers (@yarnpkg/parsers/lib/syml).