Skip to content
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

fix(nx-python): remove direct dependency on nx #255

Merged
merged 1 commit into from
Nov 25, 2024

Conversation

FrozenPandaz
Copy link
Contributor

@FrozenPandaz FrozenPandaz commented Nov 22, 2024

Current Behavior

@nxlv/python has a direct dependency on nx locked to 20.1.2. In workspaces which are not on Nx 20.1.2 (either older or newer), this causes there to be multiple versions of Nx in the workspace. Nx does its best to mitigate these sorts of issues but ideally, Nx plugins do not bring in their own version of nx.

Expected Behavior

The way it should work for Nx plugins is that they all depend on @nx/devkit. @nx/devkit itself has a peer dependency on the proper range of Nx which can be used with that particular version of @nx/devkit. For example, @nx/devkit@20.1.2 works with Nx 19 - 21. Nx Plugins should inherit this peerDependency through it's dependency on @nx/devkit rather than directly depending on nx. What this means for workspaces is that all of the plugins used in a workspace should depend on the one main version of nx specified by that workspace.

So what I've done in this PR is remove the dependency from @nxlv/python onto nx. @nxlv/python goes from having an explicit dependency on nx@20.1.2 to having a peerDependency on Nx 19 - 21. This peerDependency will be updated as you update @nxlv/python's dependency on @nx/devkit. This solves issues that users using versions of Nx other than 20.1.2 may encounter due to having multiple versions of Nx present in their workspace.

Please let me know if you have any questions. :)

Related Issue(s)

Fixes #

"ora": "5.3.0",
"semver": "^7.5.3"
"semver": "^7.5.3",
"nx": "20.1.2"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @FrozenPandaz, thanks for raising this PR.

The nx dependency still in the package.json, and I also noticed you are removing some other packages.

That doesn't feel right, could you check pls?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh no, I'm so sorry. Funnily enough, the nx dependency-check lint rule acted up for me locally. I'll take a look at that but it's not related to this PR.

I ignored the imports from the lint rule... It's still okay for you to import nx without depending on it because you have a peer dependency.. it should be resolveable. Ideally our lint rule wouldn't warn about it. Also ideally, you should not import directly from nx but we recognize that there are some private APIs which you are using. We'll take note of it and see if we can expose those as @nx/devkit utils for you to move to.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't worry mate, thanks for pushing a fix for that

@lucasvieirasilva lucasvieirasilva merged commit 762db7d into lucasvieirasilva:main Nov 25, 2024
1 check passed
github-actions bot pushed a commit that referenced this pull request Nov 25, 2024
## [20.0.1](nx-python-v20.0.0...nx-python-v20.0.1) (2024-11-25)

### Bug Fixes

* **nx-python:** remove direct dependency on nx ([#255](#255)) ([762db7d](762db7d))
Copy link

🎉 This PR is included in version 20.0.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@FrozenPandaz FrozenPandaz deleted the rm-nx-dep branch November 25, 2024 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants