Skip to content

Conversation

@olblak
Copy link
Member

@olblak olblak commented Jan 1, 2023

Signed-off-by: Olblak [email protected]

Dependson:

Add npm autodiscovery to bump any version specified as dependencies or devDependencies specified in a package.json

Updatecli ignores version bump for the following scenarios

  • any package.json in node_modules directories
  • Any version constraint handled by NPM starting with "*", ">","<","~"

Example

Manifest

Click to expand

########################################
# [NPM] BUMP @MDI/FONT PACKAGE VERSION #
########################################

name: '[NPM] Bump @mdi/font package version'
pipelineid: b1f2a3e1656341a003847d1b725c14081e2152f27166a31bd668902bc7814d15
sources:
    npm:
        name: Get @mdi/font package version
        kind: npm
        spec:
            name: '@mdi/font'
            versionfilter:
                kind: semver
                pattern: '>=5.9.55'
targets:
    npm:
        name: Bump @mdi/font package version
        kind: json
        spec:
            file: package.json
            key: dependencies.@mdi/font
        sourceid: npm

Pipeline Run

Click to expand
 ~/Projects/Updatecli/app-dashboard │ ../updatecli/bin/updatecli diff --experimental                                                                                       
Experimental Mode Enabled


+++++++++++
+ PREPARE +
+++++++++++


SCM repository retrieved: 0


++++++++++++++++++
+ AUTO DISCOVERY +
++++++++++++++++++



#######################
# LOCAL AUTODISCOVERY #
#######################

NPM
====
1 manifests identified
Manifest detected: 1

---

=> Total manifest detected: 1



++++++++++++
+ PIPELINE +
++++++++++++



#######################
# LOCAL AUTODISCOVERY #
#######################


########################################
# [NPM] BUMP @MDI/FONT PACKAGE VERSION #
########################################


SOURCES
=======

npm
---
Searching for version matching pattern ">=5.9.55"
✔ Version 7.1.96 found for package name "@mdi/font"


TARGETS
========

npm
---

**Dry Run enabled**

⚠ Key 'dependencies.@mdi/font', from file 'package.json', is incorrectly set to 5.9.55 and should be 7.1.96'

=============================

REPORTS:


✔ Local AutoDiscovery:

⚠ [NPM] Bump @mdi/font package version:
	Source:
		✔ [npm] Get @mdi/font package version (kind: npm)
	Target:
		⚠ [npm] Bump @mdi/font package version (kind: json)


Run Summary
===========
Pipeline(s) run:
  * Changed:	1
  * Failed:	0
  * Skipped:	0
  * Succeeded:	1
  * Total:	2

Test

To test this pull request, you can run the following commands:

go build .
# Then from any directories containing package.json, run
updatecli diff --experimental 
# OR
cd pkg/plugins/autodiscovery/npm/
go test
cd test 
go test

Additional Information

Tradeoff

Potential improvement

  1. I know have a lot of logic in the npm autodiscovery plugin that should be moved to a specific npm resource
  2. Allow to restrict version update to a specific type such as only major/minor/patch update e if semver is used
  3. Yarn Target updating the lockfile, when executed in dry-run mode shouldn't not change the yarn.lock`
  4. Leverage google/osv.dev to detect CVEs

Signed-off-by: Olblak <[email protected]>
@olblak
Copy link
Member Author

olblak commented Jan 1, 2023

@olblak
Copy link
Member Author

olblak commented Jan 2, 2023

npm package allows dots in name such as "highlight.js" which is not compatible with the current json key.
I need to additional testing

@olblak
Copy link
Member Author

olblak commented Jan 2, 2023

After a night thinking on this, I realize that I took a different path than Dependabot.
In this pullrequest, I assume that a version constraint specified in a package.json shouldn't be changed.
Differently said, >2.17 should not be changed by Updatecli unless we specify the flag force (not implemented yet).

On the other side, Dependabot proposes a version bump following the same version pattern so it would propose >2.18 to replace 2.17 if possible.

I do prefer to assume that a version constraint specified in a package.json has a meaning that Updatecli shouldn't override

@olblak
Copy link
Member Author

olblak commented Jan 3, 2023

I improved the crawler to add an additional target to cleanup the lock file generated by yarn or npm.
If a lockfile exists then Updatecli requires yarn or npm to be executed

targets:
    npm:
        name: Bump "@mdi/font" package version
        kind: json
        spec:
            file: package.json
            key: dependencies.@mdi/font
        scmid: default
        sourceid: npm
    package-lock.json:
        dependson:
            - npm
        name: Update NPM lockfile package-lock.json
        kind: shell
        spec:
            command: npm install --package-lock-only
        scmid: default
        disablesourceinput: true

I am now facing two additional issues

  1. The npm command must be executed from the same directory than the package.json but the shell resource do not allow to override the default workingdirectory. One option would be to add an additional shell parameter "workingdir"
  2. The shell command is always executed and therefor try to commit unchanged files which trigger the same errror than Add Git Branch resource #1000 (comment)

Working on #942 could solve the current issues

@olblak olblak added this to the 0.46.0 milestone Jan 23, 2023
@olblak olblak mentioned this pull request Jan 24, 2023
2 tasks
@olblak olblak added autodiscovery All things related to the autodiscovery feature enhancement New feature or request labels Jan 26, 2023
@olblak
Copy link
Member Author

olblak commented Jan 30, 2023

Update on my testing of this pullrequest.

The "file/checksum" shell success criteria doesn't fully solve my problem. As it checks if a file changed or not, in dry-run mode, Updatecli do not detect if a file should be changed even though the console output correctly display a change such as

Example

##################################
# BUMP "CORE-JS" PACKAGE VERSION #
##################################


SOURCES
=======

npm
---
Searching for version matching pattern "^3.8.0"
✔ Version 3.27.2 found for package name "core-js"


TARGETS
========

package-lock.json
-----------------

**Dry Run enabled**

The shell 🐚 command "/bin/sh /tmp/updatecli/bin/34fde7d5e0d61b4cf2a5aedcdd9c1aebb51d5d97ed959020c8d6ebfc1cc2c778.sh" ran successfully with the following output:
----

up to date in 718ms

103 packages are looking for funding
  run `npm fund` for details
----
No change detected

@olblak olblak modified the milestones: 0.46.0, 0.44.0 Feb 1, 2023
@olblak olblak merged commit fcef1d9 into updatecli:main Feb 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autodiscovery All things related to the autodiscovery feature enhancement New feature or request resource-npm

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants