search

jeffijoe / typesync

Install missing TypeScript typings for dependencies in your package.json.
MIT License
1.53k stars 22 forks source link

Support PNPM Monorepos #119

Closed lishaduck closed 3 months ago

lishaduck commented 4 months ago

Closes: #118.

~~Depends on #117. Diff (GH doesn't like stacked PRs from forks): https://github.com/lishaduck/typesync/compare/fix-monorepos...pnpm~~

lishaduck commented 4 months ago

Note: I haven't added tests yet. I wanted to make sure this implementation was fine first. I'm not a big fan of adding code about a different file to a service about package.jsons, but fixing that seemed a bit like the diff might... erm... explode. So I just pretended pnpm-workspace was a package.json file.

lishaduck commented 4 months ago

Forgot to remove a console.log. I'll get it :)

jeffijoe commented 4 months ago

Could you rebase this PR? I merged the other one!

coveralls commented 4 months ago

Coverage Status

coverage: 100.0%. remained the same when pulling 18605181c3d5dfc689658947b3ec316c72ab3a73 on lishaduck:pnpm into 7aecb96b101fee4fc6f82c9f90776cc997710967 on jeffijoe:master.

lishaduck commented 4 months ago

Yay! I found a bug in npm 🤣

lishaduck commented 4 months ago

Currently, it works... well, CI fails.

I was going to say... It works, but it's not polished. It currently grabs all the package.json files multiple times at least. I did drop some old polyfills at least 😁

lishaduck commented 4 months ago

Ok, it works locally, but apparently Node 22.5 broke npm: npm/cli#7657

lishaduck commented 4 months ago

Almost to a point I'm happy. I've got it reusing the package.json, now I just want to extract out fs to an interface so I can just mock the fs in tests instead of the cod, which should help readability (and code coverage).

And then I'll need to clean up the history a bit more...

lishaduck commented 4 months ago

Looks good! Just gotta get CI to pass now.

Yup. I'll also need to get some tests (which I suppose is needed for CI). Can you revert node to 22.4.x? 22.5.x still hasn't had a patch for npm.

jeffijoe commented 4 months ago

CI uses current so it'll always pull the latest. Let's give it another day, I'm sure they'll release a patch soon.

lishaduck commented 4 months ago

CI uses current so it'll always pull the latest. Let's give it another day, I'm sure they'll release a patch soon.

Ok, fair enough. See ya then! (I'm working through tests. Almost done (I hope))

lishaduck commented 4 months ago

Added some tests, should be 100% coverage, but I don't have a good way of checking.

jeffijoe commented 4 months ago

Added some tests, should be 100% coverage, but I don't have a good way of checking.

npm run cover

that should do it

lishaduck commented 4 months ago
Screenshot 2024-07-18 at 8 35 03 PM

How is this line untested? https://github.com/jeffijoe/typesync/blob/c69636bbc0c0d98818cfcae15638fd6f099fe66a/src/workspace-resolver.ts#L95

It's just a function declaration. And it's causing branch coverage problems? What?

lishaduck commented 4 months ago

Ok, that was weird. I guess parameter defaults get considered as the declaration line, and branching %s factor in how high into the call stack it is???

jeffijoe commented 4 months ago

Were you able to get the last branch covered?

Also, looks like the bug has been fixed, we're just waiting for a release.

lishaduck commented 4 months ago

Were you able to get the last branch covered?

Yup. 18605181c3d5dfc689658947b3ec316c72ab3a73

Also, looks like the bug has been fixed, we're just waiting for a release.

Yup. https://nodejs.org/en/blog/release/v22.5.1

Can I get a CI rerun?

lishaduck commented 3 months ago

@jeffijoe, ping :) Can you retrigger ci?

jeffijoe commented 3 months ago

Thanks! I’ll release this tomorrow. :) nice work!

lishaduck commented 3 months ago

Thanks! I’ll release this tomorrow. :) nice work!

Thanks to you! It was a pleasure to write, nicely architectured (I'll admit I'm not a big fan of dependency injection containers, but they do serve a purpose). It was lots of fun, entirely worthwhile, and feel free to ask me to fix monorepo support again if it breaks and you're not in mood to fix it yourself becuase I actually want to find an excuse to write some more code here 😉[^1]

[^1]: And, of course, it'll come at the wrong time and I'll regret saying so. But for now, feel free. 😜