feat: add support for yarn 2 lock files #56 #57

regevbr · 2020-04-14T19:36:31Z

Tests written and linted
Documentation written / README.md updated https://snyk.io/docs/snyk-for-node/
Follows CONTRIBUTING agreement
Commit history is tidy https://git-scm.com/book/en/v2/Git-Branching-Rebasing
Reviewed by Snyk team

What this does

Fixes #56
Allows parsing of yarn 2 lock files, which are different from yarn 1

Notes for the reviewer

I used @yarnpkg/parsers to parse the lock file
I used @yarnpkg/core for parsing the version descriptors that are in the lock files
yarn 2 is only supported from node 10 and above (I didn't compromise the compatibility of this library)
I reused the yarn 1 parser logic, by normalizing the dep tree from yarn 2
I fixed some existing tests that were wrong
To realize that the project is using yarn 2, I check the existence of .yarnrc.yml
I had to upgrade to typescript 3.4 (the lowest possible version) as the typings in the dependencies of yarns packages were causing build issues and the only way to ignore them (as they are not important) is to use tsconfig skipLibCheck, which was only introduced (or at least worked) in typescript 3.4

CLAassistant · 2020-04-14T19:36:38Z

All committers have signed the CLA.

regevbr · 2020-04-14T19:52:55Z

@orsagie @lili2311 @FauxFaux @miiila can you please CR this?

lili2311 · 2020-04-15T09:01:06Z

Hi @regevbr 👋 Thank you was this awesome contribution, we have many team members on holiday today so we would like to discuss this internally with them when they are back tomorrow and get back to you.

regevbr · 2020-04-20T13:19:55Z

@lili2311 how are you?

Can you please let me know when you are expected to fix the yarn2 issue? this is the only reason I can't migrate my projects to use yarn2...

regevbr · 2020-04-20T18:05:46Z

@arcanis I know you are a very busy man, but I was wondering if you can take a quick look into my PR here to see if I have done it right and if maybe there is a better way to achieve it?
I thought about resuing the code from each relevant plugin but they are too coupled to the actual resolving logic.

arcanis

I didn't run the tests, just looking at the Yarn-specific parts 🙂

lib/index.ts

arcanis · 2020-04-20T18:27:22Z

lib/parsers/yarn-lock-parse-base.ts

+        strict = true ): Promise<PkgTree> {
+        if (lockfile.type !== this.type) {
+            throw new InvalidUserInputError('Unsupported lockfile provided. ' +
+                'Please provide `package-lock.json`.');


Suggested change

'Please provide `package-lock.json`.');

'Please provide a `yarn.lock` file.');

I will leave that decision to the maintainers

lib/parsers/yarn-utils.ts

lib/parsers/yarn2-lock-parse.ts

regevbr · 2020-04-20T18:44:01Z

@arcanis thanks for the review! much appreciated :-) I will fix all your comments

lili2311 · 2020-04-21T13:44:18Z

lib/index.ts

@@ -79,7 +90,11 @@ async function buildDepTreeFromFiles(
  if (_.endsWith(lockFilePath, 'package-lock.json')) {
    lockFileType = LockfileType.npm;
  } else if (_.endsWith(lockFilePath, 'yarn.lock')) {
-    lockFileType = LockfileType.yarn;
+    if (fs.existsSync(path.resolve(root, lockFilePath.replace('yarn.lock', '.yarnrc.yml')))) {


is there anywhere else these can live? they are always in the same directory?

No, they are in the same directory

lili2311 · 2020-04-21T13:44:39Z

lib/index.ts

+    if (fs.existsSync(path.resolve(root, lockFilePath.replace('yarn.lock', '.yarnrc.yml')))) {
+      lockFileType = LockfileType.yarn2;
+    } else {
+      lockFileType = LockfileType.yarn;


Note: we need a way to send some meta back to track yarn v2 tests vs others

The lock file is sent back in the return value of buildDepTree. You mean you need to handle the different type in the code that uses that library?

lili2311 · 2020-04-21T13:44:45Z

lib/index.ts

+      } else {
+        throw new UnsupportedRuntimeError('Parsing `yarn.lock` is not ' +
+          'supported on Node.js version less than 10. Please upgrade your ' +
+          'Node.js environment or use `package-lock.json`');


in this situation an older v1 yarn.lock would also work? Doesn't make sense to ask to switch to npm

It will not, as the lock file in v1 is not standard yaml file... And in any case yarn2 only works with node 10 and above so I don't think we will have a problem here

dkontorovskyy · 2020-04-22T11:20:25Z

@regevbr that is really nice contribution. I will pull it to test locally first and then let's try to merge it. If I will need any of your help, are you up to pairing in close future?

regevbr · 2020-04-22T11:23:52Z

@dkontorovskyy sure thing! anything you need. Thanks!

regevbr · 2020-04-23T10:55:02Z

I rebased as there were some changes made to master.
I also synced the tree size limits logic to the base yarn parser and added the relevant test to yarn2 parser test

dkontorovskyy · 2020-04-27T08:07:05Z

Hey @regevbr. I run local tests on your PR and it looks great 🎉 . I would like to make a graceful roll out for yarn v2, so it would be great if we can pair and separate this PR in 2 smaller bits and take it 1 by 1 into this plugin. I would like to schedule a pairing session with you whenever you free. What do you think about that?

regevbr · 2020-04-27T12:10:55Z

@dkontorovskyy thanks!
Sure we can do a session, I'm generally available 12am-3am GMT+3
How do you want to split it?

dkontorovskyy · 2020-04-27T12:14:40Z

@regevbr luckily we are in the same timezone 😃
I can do 12AM-2PM tomorrow. Want me to send you an invite to zoom meeting? How can I contact you ?
I would like to split it into two: first one is refactor needed for a new feature and second one is actually yarn2 support that will be easily added once the first one is in.

dkontorovskyy · 2020-05-11T07:42:11Z

@regevbr just keeping update public 😃 . We are looking to plan this feature properly this sprint and start active development next sprint. This is an amazing contribution, so we would try to bring it into the core of Snyk as soon as possible, we just need to make sure this can be done harmlessly to the overall system 🙏

lili2311 · 2020-05-18T13:30:47Z

#56

dkontorovskyy · 2020-05-28T08:03:21Z

@regevbr first refactor PR went in 💪

regevbr · 2020-05-28T08:07:42Z

@dkontorovskyy that's good news!
I will rebase this branch so we will be ready for the next phasse

dkontorovskyy · 2020-06-01T08:23:47Z

@arcanis using @yarnpkg/core in module yields tsc error

node_modules/@types/emscripten/index.d.ts:53:33 - error TS2304: Cannot find name 'WebGLRenderingContext'.
53     preinitializedWebGLContext: WebGLRenderingContext;
                                   ~~~~~~~~~~~~~~~~~~~~~
node_modules/@types/emscripten/index.d.ts:67:28 - error TS2304: Cannot find name 'MessageEvent'.
67     onCustomMessage(event: MessageEvent): void;

I see that there was an addition to package.json of @yarnpgk/libzip to use @types/emscripten as production dependencies, because of the bug (yarnpkg/berry#746).

Loos like node.js doesn't like WebAssembly types.

Are you aware of any workarounds for that except silencing TSC compiler with --skipLibCheck ?

regevbr · 2020-06-01T10:48:36Z

@dkontorovskyy this is solved by adding "dom" as a lib to tsconfig:

        "lib": [
            "es2017",
            "dom"
        ],

I tried getting the library to compile without skipping the lib check, and it requires a lot of changes as there are multiple errors originating from different modules.
The only errors left comes from d.ts reference issues in @yarnpkg/core. For example:

node_modules/@yarnpkg/core/lib/structUtils.d.ts:91:66 - error TS2307: Cannot find module '../../yarnpkg-fslib/sources'.

91 export declare function slugifyLocator(locator: Locator): import("../../yarnpkg-fslib/sources").Filename;

I will open a ticket to fix it in yarn. You can check out all the changes I made in the next commit to this branch and let me know if that is that important for you to not skip the lib check.

regevbr · 2020-06-14T18:04:46Z

@dkontorovskyy I have worked with @arcanis and the issue is now fixed!
There is a new issue in clipanion which I will try to get resolved soon as well

regevbr · 2020-06-15T08:52:30Z

@dkontorovskyy I fixed the issue in clipanion and now the build works perfectly!
How do we proceed?

dkontorovskyy · 2020-06-16T12:59:28Z

@regevbr that is so cool!! What was the issue and fix?

regevbr · 2020-06-16T13:33:53Z

@dkontorovskyy in clipanion? see arcanis/clipanion#25 for more details, in high level, the tool used for minifying the d.ts file has bugs in it causing a faulty d.ts file.

If you are asking about the issue in @yarnpkg/core than it is very complicated you can check out my
summary at yarnpkg/berry#1278 (comment)

dkontorovskyy · 2020-06-24T10:25:12Z

@regevbr I merged your PR in our separate feature branch to run some final round of investigation we are doing with the team today. Will keep you posted 🤗

regevbr · 2020-06-24T10:37:48Z

Thanks @dkontorovskyy!
Looking forward to hear back from you

regevbr · 2020-07-12T09:42:37Z

@dkontorovskyy! how's it going? I see that the feature was finally merged! does that mean I can enable snyk in my yarn2 packages now (ci and PR guard)?

regevbr · 2020-07-28T11:12:43Z

@dkontorovskyy! how's it going? I see that the feature was finally merged! does that mean I can enable snyk in my yarn2 packages now (ci and PR guard)?

dkontorovskyy · 2020-08-06T09:44:09Z

Hi @regevbr, sorry for spacing out!

It should work in CLI now. Just try scanning your project locally or in CI. Not in PR guard yet. My team will be looking soon into this.

regevbr · 2020-08-06T11:38:33Z

That's great news! Please let me know when the guard is working as well as we use it as well :-)

kachkaev · 2020-12-24T11:41:06Z

Still getting parse errors in a Github PR here: kachkaev/njt#29

What are the current plans to resolve this?

I had to replace Snyk integration with running yarn audit on schedule in another project because it was blocking the PR merge. Looking forward to bring Snyk back when CI integration is finally working!

cc @arcanis

kachkaev · 2021-08-07T11:46:30Z

👋 from August 2021 😅

kachkaev/njt#29

kachkaev · 2021-11-21T13:59:53Z

UPD: 👋 from November 2021 😅

https://github.com/snyk/snyk/issues/1518#issuecomment-974822508

regevbr requested a review from a team as a code owner April 14, 2020 19:36

regevbr force-pushed the #56 branch from f362795 to 5ef2940 Compare April 14, 2020 19:44

regevbr mentioned this pull request Apr 14, 2020

chore: add tests showing we don't support yarn2 yet #54

Closed

5 tasks

regevbr force-pushed the #56 branch 2 times, most recently from 271fec5 to 0e9c897 Compare April 14, 2020 23:07

regevbr changed the title ~~bug: yarn 2 lock file parsing issues #56~~ feat: yarn 2 lock file parsing issues #56 Apr 14, 2020

regevbr changed the title ~~feat: yarn 2 lock file parsing issues #56~~ feat: add support for yarn 2 lock files #56 Apr 14, 2020

regevbr force-pushed the #56 branch from 0e9c897 to bd2b342 Compare April 14, 2020 23:07

arcanis reviewed Apr 20, 2020

View reviewed changes

lili2311 reviewed Apr 21, 2020

View reviewed changes

regevbr force-pushed the #56 branch from 6141152 to 9efd92b Compare April 23, 2020 10:49

regevbr force-pushed the #56 branch from 9efd92b to fb0f1dd Compare April 27, 2020 12:09

dkontorovskyy mentioned this pull request Apr 28, 2020

Feat/get ready for yarn2 #69

Merged

5 tasks

regevbr force-pushed the #56 branch from fb0f1dd to 008108c Compare May 28, 2020 08:34

dkontorovskyy mentioned this pull request Jun 1, 2020

chore: update nodejs-lockfile-parser that contain pre-yarn2 refactor snyk/cli#1155

Closed

regevbr mentioned this pull request Jun 1, 2020

[Bug] @yarnpkg/core definitions uses ../../yarnpkg-fslib/sources yarnpkg/berry#1278

Closed

1 task

regevbr mentioned this pull request Jun 14, 2020

bug: ts issues in d.ts file arcanis/clipanion#25

Closed

regevbr force-pushed the #56 branch from 8792dd4 to c8cef7a Compare June 14, 2020 18:03

regevbr force-pushed the #56 branch from c8cef7a to 4859e81 Compare June 15, 2020 08:48

feat: add support for yarn 2 lock files

6fd8cf9

regevbr force-pushed the #56 branch from 4859e81 to 6fd8cf9 Compare June 15, 2020 08:51

dkontorovskyy changed the base branch from master to feat/yarn-v2 June 24, 2020 10:20

dkontorovskyy merged commit cbdb8fa into snyk:feat/yarn-v2 Jun 24, 2020

anthogez mentioned this pull request Jun 24, 2020

feat: add support yarn v2 snyk/cli#1234

Merged

3 tasks

regevbr deleted the #56 branch August 6, 2020 11:37

arcanis mentioned this pull request Dec 21, 2020

Towards yarn v3 (berry) kachkaev/njt#29

Closed

10 tasks

	'Please provide `package-lock.json`.');
	'Please provide a `yarn.lock` file.');

feat: add support for yarn 2 lock files #56 #57

feat: add support for yarn 2 lock files #56 #57

Conversation

regevbr commented Apr 14, 2020 • edited Loading

What this does

Notes for the reviewer

CLAassistant commented Apr 14, 2020 • edited Loading

regevbr commented Apr 14, 2020

lili2311 commented Apr 15, 2020

regevbr commented Apr 20, 2020

regevbr commented Apr 20, 2020

arcanis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

regevbr commented Apr 20, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dkontorovskyy commented Apr 22, 2020

regevbr commented Apr 22, 2020

regevbr commented Apr 23, 2020

dkontorovskyy commented Apr 27, 2020

regevbr commented Apr 27, 2020

dkontorovskyy commented Apr 27, 2020

dkontorovskyy commented May 11, 2020

lili2311 commented May 18, 2020

dkontorovskyy commented May 28, 2020

regevbr commented May 28, 2020

dkontorovskyy commented Jun 1, 2020 • edited Loading

regevbr commented Jun 1, 2020 • edited Loading

regevbr commented Jun 14, 2020

regevbr commented Jun 15, 2020

dkontorovskyy commented Jun 16, 2020

regevbr commented Jun 16, 2020

dkontorovskyy commented Jun 24, 2020

regevbr commented Jun 24, 2020

regevbr commented Jul 12, 2020

regevbr commented Jul 28, 2020

dkontorovskyy commented Aug 6, 2020

regevbr commented Aug 6, 2020

kachkaev commented Dec 24, 2020

kachkaev commented Aug 7, 2021

kachkaev commented Nov 21, 2021

regevbr commented Apr 14, 2020 •

edited

Loading

CLAassistant commented Apr 14, 2020 •

edited

Loading

dkontorovskyy commented Jun 1, 2020 •

edited

Loading

regevbr commented Jun 1, 2020 •

edited

Loading