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!: publish shrinkwrap with prod dependencies only #5547

Merged
merged 2 commits into from
Apr 14, 2023
Merged

Conversation

danez
Copy link
Contributor

@danez danez commented Mar 9, 2023

Summary

Ref #5323

This is a reattempt of #5359 which fixes the problems that occurred last time.

It removes the shrinkwrap from the repo and instead uses the normal package-lock.

On publish we remove devDependencies, scripts & ava properties from package.json and then create the shrinkwrap. This new way ensures that all optional dependencies are present in the shrinkwrap file. (Last time the optional dependencies were missing)

Also split up the release workflow into 2 jobs so we do not have to repeat the if: ${{ steps.release.outputs.release_created }} for every single step.

@danez danez self-assigned this Mar 9, 2023
@github-actions
Copy link

github-actions bot commented Mar 9, 2023

📊 Benchmark results

Comparing with 7215229

Package size: 302 MB

⬇️ 0.00% decrease vs. 7215229

^          306 MB  306 MB  306 MB  306 MB  313 MB  313 MB  313 MB  313 MB  313 MB  313 MB                 
│  302 MB   ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐   303 MB  302 MB 
│   ┌──┐    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    ┌──┐    ┌──┐  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
└───┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴──>
    T-12    T-11    T-10    T-9     T-8     T-7     T-6     T-5     T-4     T-3     T-2     T-1      T    
Legend

@danez danez force-pushed the no-dev-dependencies branch from a40e99e to caf4c02 Compare March 9, 2023 19:50
@danez danez requested a review from lukasholzer March 15, 2023 10:45
@danez danez marked this pull request as ready for review March 15, 2023 10:45
@danez danez force-pushed the no-dev-dependencies branch from caf4c02 to 6b3d743 Compare March 15, 2023 10:46
@danez danez force-pushed the no-dev-dependencies branch from 6b3d743 to ce5f2e6 Compare March 22, 2023 14:23
text: 'Running `npm shrinkwrap`',
}).start()
await execa('npm', ['shrinkwrap'])
spinner.succeed()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the main change. This script will run before publishing (see package.json).

@EverlastingBugstopper
Copy link

Thanks for the work on this!

@danez danez force-pushed the no-dev-dependencies branch 3 times, most recently from a38b954 to ac48a98 Compare April 13, 2023 18:07
@danez danez force-pushed the no-dev-dependencies branch from ac48a98 to 89fcffe Compare April 13, 2023 18:08
@danez danez requested review from eduardoboucas and lukasholzer and removed request for lukasholzer April 13, 2023 18:09
Copy link
Collaborator

@lukasholzer lukasholzer left a comment

Choose a reason for hiding this comment

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

Should we mark it as a breaking change?

@danez danez changed the title fix: publish shrinkwrap with prod dependencies only fix!: publish shrinkwrap with prod dependencies only Apr 14, 2023
@danez danez merged commit fc38644 into main Apr 14, 2023
@danez danez deleted the no-dev-dependencies branch April 14, 2023 15:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants