-
-
Notifications
You must be signed in to change notification settings - Fork 50
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
feat: find mismatches in internal monorepo packages #66
Comments
Released in 7.0.0 |
Tested this out, does this look like the correct way to use the command package.json#19?:
I manually changed on of the internal dependencies to be wrong workspaces/templates/packages/serverless-api/package.json Ideally using syncpack this should be changed from Did I use the command wrong? Thank you!! |
On mobile now but that seems right, I will take a look at that branch soon and try to reproduce and fix, thanks. |
@mxro would you mind forking and adding your scenario to this file? I'll pick up from there and add a test and fix. Hopefully it makes sense 🤞🏻, the tests that use these mock scenarios look a bit like this but I'll do those, I just need a reduced scenario that matches your situation then I can take a look 👍🏻 |
Added a draft PR - but very low confidence on my part whether that is right: #74 (I think there is also an option in GitHub to mark PRs as drafts but I couldn't find / missed the option) |
Thanks Max appreciate it |
I've added tests for your scenarios but they're passing, so I'm still not sure what the problem might be. The work on this can be seen at https://github.com/JamieMason/syncpack/compare/mxro-add_scenario. Maybe it's a problem with parsing combinations of |
I think I've managed to reproduce it in github actions here, it looks like it's a problem with syncpack's compatibility with Windows. Parking this for now but will come back to it soon. |
Ah yes that could be the case! I was running the command on Windows! Thanks for looking into it! |
If you could check out that branch and try taking a look on Windows @mxro that would be a huge help, I spent ages on Sunday messing around trying to get a Windows VM running and got nowhere. It's something I've had working before but I was hitting errors this time I've never seen before, so it'll take some time. I think the problem is around paths being different on windows, probably |
Thanks for looking into this and happy to help! With which version should I try - I think on npm it is still the same one I tried with last time |
If you can clone the mxro-add_scenario branch and run the tests on Windows that would be great, and try to see if you can apply fixes. I'll do this too once I can finally get a Windows vm working. |
Roger that, got a few test failures And here the full log:
|
Never used this before but maybe using something like https://aws.amazon.com/workspaces/?nc=sn&loc=0 maybe easier than trying to get a windows up and running locally? |
Good as a plan B but I'll get VirtualBox sorted hopefully. I've had it working before but was having trouble this time for some reason - maybe the latest Mac OS version or something, I don't know yet. |
Closing this again as the issue we were discussing when reopening was actually windows support generally – finding mismatches in internal monorepo packages was working on other OSes. |
Just ran this again on a branch: goldstack/goldstack#184 using both latest syncpack@7 and syncpack@8. Still it does not seem to be picking up the dependencies that should be fixed, eg. Here the command I am trying to run: |
Is that on Windows? Or also on Linux or osx? |
Running this on Windows - maybe let me try running it in Linux env and see what the result is! |
Seems like the result is the same when running in GHA using https://github.com/goldstack/goldstack/runs/6716718149?check_suite_focus=true#step:6:1 |
Perfect, this will be easier to debug and suggests a real code problem rather than something subtle with environments. Hopefully it just means that the test scenario we made possibly doesn't reproduce the issue correctly, we'll see. Thanks for setting that up. |
I have Without quoting the patterns, the only files being operated on were:
Without quotessyncpack list-mismatches \
--workspace \
-p \
-d \
--source package.json \
--source workspaces/*/package.json \
--source workspaces/*/packages/*/package.json ↓↓↓
With quotessyncpack list-mismatches \
--workspace \
-p \
-d \
--source 'package.json' \
--source 'workspaces/*/package.json' \
--source 'workspaces/*/packages/*/package.json' ↓↓↓
I'm doing some work to improve this, but in the meantime adding quotes around your sources should hopefully get it working 🤞🏻 |
Excellent, thanks for looking into this! Sorry I think I tried it initially with quotes but then forget to do that for the new version. Maybe the Yarn shell interferes here so it could be that nothing needs to change for syncpack. Works perfectly now and helped me do a good clean up of the repository. Also could configure it to support multiple versions of the same package I needed using version groups. And could use the list-mismatches in the CI workflow, since it will return https://github.com/goldstack/goldstack/blob/master/.github/workflows/branch.yml#L65 |
You're welcome. A lot of nice ideas and improvements have come off the back of your feedback and repros, so thanks a lot for that. |
* Update Update devDependencies (non-major) * Use workspace versions when checking dependency mismatches See: JamieMason/syncpack#66 * Add changeset Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: Emanuel Tesar <[email protected]>
Hey @JamieMason , I'm really glad you added this feature! However, in the case like ours where we have some misalignment, this is breaking. Is there a way to make this opt in only for the 8.x versions going forward and have it enabled by default in 9.x? That will give folks like me time to turn it on and fix our stuff. Otherwise for now I have to operate on 8.0.0 because this feature will block our development (or I turn it off and fix all the things) |
Hey @Aghassi, This might be enough: {
"workspace": false
} Otherwise do: {
"dev": true,
"overrides": true,
"peer": true,
"pnpmOverrides": true,
"prod": true,
"resolutions": true,
"workspace": false
} or the command line:
Shout if I've misunderstood anything. I'm reading this that you want to skip checking workspace versions, the new Thanks |
Yep you are right! Let me add that. Just for future releases, as a consumer, it would be better if minors didn't have potentially breaking changes is all. Thanks so much! Looking forward to actually using this feature! |
|
Hmmm.. well we were on that version so I'll look into why I tripped over it and get back to you. Maybe we are an edge case :P |
Yeah there could've been some regression, please make a new issue if so, thanks. |
Description
@this-monorepo/atoms
and@this-monorepo/molecules
are developed in your monorepo.packages/atoms/package.json
containspackages/molecules/package.json
containssyncpack
does not report any mismatches because nothing else in the monorepo depends on@this-monorepo/atoms
.Suggested Solution
Add
--workspace
to the existing list of locations to check for mismatches:-p, --prod include dependencies -d, --dev include devDependencies -P, --peer include peerDependencies -R, --resolutions include resolutions (yarn) -o, --overrides include overrides (pnpm) + -w, --workspace include locally developed package versions
The text was updated successfully, but these errors were encountered: