npm libs in version catalogs #671
Conversation
Vampire
force-pushed
the
npm-libs-in-version-catalogs
branch
from
74ff323
to
27fbc94
Compare
| internal fun Dependency.matches(moduleId: ModuleId): Boolean { | ||
| return moduleId.group == group && moduleId.name == name |
There was a problem hiding this comment.
This leads to an allocation on every single matches check, that's the reason why I didn't just use the conversion trick.
Could you revert this change?
There was a problem hiding this comment.
No I can not, otherwise it does not work anymore, that was not just a cosmetic change:
moduleId.group == group && moduleId.name == name
actions == null && cache == @actions/cache
moduleId == moduleId()
Npm(group=actions, name=cache) == Npm(group=actions, name=cache)
I can try to reprogram it so that still the necessary calculation is done but no allocation necessary maybe.
There was a problem hiding this comment.
Forgot to mention that I did. :-)
| this::class.simpleName == "NpmDependency" -> { | ||
| return versionsCatalogLibraries.any { | ||
| val moduleId = npmModuleId() | ||
| it.module.group == (moduleId.group ?: "<unscoped>") && it.module.name == moduleId.name |
There was a problem hiding this comment.
This "<unscoped>" thing is a a convention you defined, that isn't standard, right?
Isn't it possible to just have no group (i.e. null) instead?
There was a problem hiding this comment.
Yes, it is a convention I invented and also suggested in the mentioned https://youtrack.jetbrains.com/issue/KT-56416/KJS-Gradle-npm-compatibility-with-Gradle-version-catalogs ticket.
I hope for it to become the official standard once that ticket maybe gets attention.
Just having semver = { module = "semver", version.ref = "semver" } does not work, as the version catalog parser complains it is not valid syntax.
Same for semver = { module = ":semver", version.ref = "semver" }, invalid syntax.
So you need the colon and you need something before it.
semver = { module = "null:semver", version.ref = "semver" } would be valid in the version catalog, but actually "null" would be a valid scope too and could also easily confuse users, so I decided to use something that clearly says it is an unscoped package and is very unlikely to be used as actual scope if it is even a valid scope name, hence "<unscoped>".
There was a problem hiding this comment.
What I would hope is for Gradle to allow null group as they do when not using version catalogs, that'd cut down the noise. On the other hand, unscoped dependencies are not that great of a thing, so maybe making it super obvious might be a good thing… 🤔
Vampire
force-pushed
the
npm-libs-in-version-catalogs
branch
from
27fbc94
to
056e1a8
Compare
Vampire
force-pushed
the
npm-libs-in-version-catalogs
branch
from
056e1a8
to
519e0cf
Compare
What?
Handle version for NPM libs that are defined in a version catalog.
Why?
So that you get version update comments for NPM libs in the version catalog.
How to define and use them can be seen at https://youtrack.jetbrains.com/issue/KT-56416/KJS-Gradle-npm-compatibility-with-Gradle-version-catalogs
How?
Most things were in place already, I just needed to adjust 3 spots to handle NPM dependencies accordingly.
Testing?
Manually with my setup-wsl project.