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

chore: generate shrinkwrap without including dev and peer deps #2189

Merged

Conversation

yaumu3
Copy link
Contributor

@yaumu3 yaumu3 commented Nov 6, 2023

Intent

Exclude dev and peer dependencies from npm-shrinkwrap.json upon release. As of v5.8.1, they are included (Many "dev": true exist in npm-shrinkwrap.json).

Confirmed

By following the updated workflow manually, confirmed no dev/peer dependencies are included in npm-shrinkwrap.json.

@yaumu3
Copy link
Contributor Author

yaumu3 commented Nov 6, 2023

If this PR looks sensible, I'd like to submit the similar ones for UIA2 and Espresso driver.

@yaumu3 yaumu3 marked this pull request as ready for review November 6, 2023 05:47

# Generate shrinkwrap from the content of node_modules
mv package-lock.json package-lock.json.bak
npm shrinkwrap
Copy link
Member

Choose a reason for hiding this comment

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

mm, it seems like this command puts "dev" in the npm-shrknkwrap.json? Let me check a few further later

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe the behaviour has changed in npm itself as we use the latest LTS node

Copy link
Member

Choose a reason for hiding this comment

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

Checked a few additionally. Actually npm included in over node 18 (so lockfileVersion v3 thing?) generated dev included one. Thus currently no good way to include only dependencies without dev attribute in the published package, while actually the lock file with --omit (in current our publish.yml way) did not include everything in the dev dependencies, thus not bad

Copy link
Member

@KazuCocoa KazuCocoa left a comment

Choose a reason for hiding this comment

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

Actually this helped to not save dev env in the npm-shrinkwrap.json, but probably other drivers that haven't had package-lock.json won't change so much. To expand this to other drivers, them maybe only the --omit option can help to use newer syntax for the same purpose

@KazuCocoa
Copy link
Member

let me merge and see how this will be

@KazuCocoa KazuCocoa merged commit c19c1a9 into appium:master Nov 8, 2023
17 checks passed
@yaumu3 yaumu3 deleted the remove-dev-dependencies-from-npm-shrinkwrap branch November 8, 2023 08:35
Copy link
Contributor

github-actions bot commented Nov 8, 2023

🎉 This PR is included in version 5.8.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

@yaumu3
Copy link
Contributor Author

yaumu3 commented Nov 8, 2023

Thank you!

but probably other drivers that haven't had package-lock.json won't change so much.

Roughly checked on appium-uiautomator2-driver and it seems so..

@KazuCocoa
Copy link
Member

KazuCocoa commented Nov 8, 2023

Interestingly appium/appium-geckodriver#89 generated non-dev in npm-shrinkwrap.json. Probably the same change can apply for other drivers in this org repos, then can get the same/similar benefit

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.

3 participants