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.

Sign up for GitHub

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

delete lock file when package is updated #179

Closed
wants to merge 3 commits into from
Closed

delete lock file when package is updated #179

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
55cd822
delete lock file when package is updated
Steve-Mcl Sep 27, 2023
62176e2
move package lock deletion to own fn (for testing)
Steve-Mcl Oct 5, 2023
23327fc
update test to ensure package lock is deleted
Steve-Mcl Oct 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions lib/launcher.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ class Launcher {

this.files = {
packageJSON: path.join(this.projectDir, 'package.json'),
packageLockJSON: path.join(this.projectDir, 'package-lock.json'),
flows: path.join(this.projectDir, 'flows.json'),
credentials: path.join(this.projectDir, 'flows_cred.json'),
settings: path.join(this.projectDir, 'settings.js'),
Expand All @@ -54,6 +55,13 @@ class Launcher {
await fs.writeFile(this.files.packageJSON, JSON.stringify(packageData, ' ', 2))
await fs.rm(path.join(this.projectDir, '.config.nodes.json'), { force: true })
await fs.rm(path.join(this.projectDir, '.config.nodes.json.backup'), { force: true })

// as the package.json file has been re-written, we will "assume" different versions
// of the modules and therefore remove the package-lock.json file to permit modules
Copy link
Contributor

Choose a reason for hiding this comment

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

It may be worth noting that this is only because of the --production flag, a normal npm install would update the package-lock.

// to be updated to the versions specified in package.json
if (existsSync(this.files.packageLockJSON)) {
await fs.rm(this.files.packageLockJSON)
Copy link
Contributor

@Pezmc Pezmc Sep 28, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Wouldn't an npm install already handle this?

Or are you wanting to run npm upgrade to upgrade the package lock versions specifically to the latest matching the semver? (e.g. 1.2.3 -> 1.2.5 for a ^1.2.0)

Copy link
Contributor Author

@Steve-Mcl Steve-Mcl Oct 5, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Pez, was discussed in slack

Steve
When switching snapshots with different versions of Node-RED specified, "Installing Packages" is ran but Node-RED version remains old.
npm install --production (as the launcher and agent execute) ignore package.json and respect the lock file instead
Should we be inspecting the package or indiscriminately deleting the lock file in the agent device project dir when "Updating configuration files"-> writePackage() is called?
2 replies

nick
My instinct is to delete the package lock file whenever we update the package file

Steve
same - but wanted to check

Copy link
Contributor

Choose a reason for hiding this comment

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

To me, this could lead to unexpected upgrades outside of the package changes, but so would running an explicit npm upgrade, so unblocking until we find a better solution for now.

Copy link
Contributor

Choose a reason for hiding this comment

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

On second thought, this still doesn't fully grok for me actually, an npm install always updates the package-lock based on the latest package.json as part of the install. Testing locally.

Copy link
Contributor

Choose a reason for hiding this comment

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

Result of my tests in #178 (comment)

}
}

async readPackage () {
Expand Down