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

Bump package-lockfile to v3 #7099

Merged
merged 2 commits into from
Aug 18, 2024
Merged

Conversation

birkskyum
Copy link
Contributor

@birkskyum birkskyum commented Aug 13, 2024

No description provided.

@archmoj
Copy link
Contributor

archmoj commented Aug 13, 2024

What are the benefits of a lock file v3?

@birkskyum
Copy link
Contributor Author

birkskyum commented Aug 13, 2024

Mainly that it's half the size, which makes it faster to read/modify, and reduces conflict

@archmoj
Copy link
Contributor

archmoj commented Aug 13, 2024

What are the minimum npm & node required versions that support lock file v3?

@birkskyum
Copy link
Contributor Author

birkskyum commented Aug 13, 2024

I believe it's minimum npm v9. Node 18 comes with npm 10, and I think node 16 comes with npm 8
UPDATE: npm v8 (and v7) can consume it as well, so it's all good.

@archmoj
Copy link
Contributor

archmoj commented Aug 13, 2024

I'm afraid this could be considered a breaking change.
But that's definitely something to think about for plotly.js v3.
cc: @ndrezn @gvwilson

@birkskyum
Copy link
Contributor Author

Okay. It'll not change the bundle one byte though, so I don't see how it can break for any users.

@archmoj
Copy link
Contributor

archmoj commented Aug 13, 2024

Okay. It'll not change the bundle one byte though, so I don't see how it can break for any users.

We have users that use this custom bundle script to generate custom bundles. If they still use node.js v16 in their system I think we should allow them to continue for a bit longer. No?

@alexcjohnson What do you think?

@birkskyum
Copy link
Contributor Author

birkskyum commented Aug 13, 2024

Aha, intersting. @archmoj I've just tried with npm v8 (shipped with node 16), and fortunately it's perfectly capable of consuming a lockfile v3.

Even npm v7 can consume it, it just rewrites it to a lockfile v2, but it does not break.

@alexcjohnson
Copy link
Collaborator

Given that we can support users back to npm v7 (older node 16 versions) I think it's reasonable to make this switch now and not consider it a breaking change. It'll require contributors to be on later node/npm versions at least if they're touching dependencies, but that's fine, we've never considered contributor requirement changes as breaking. Also I'll note we already describe later node/npm as a requirement for custom bundling:

Make sure you have the versions of node/npm that's recommended:
- plotly.js before 2.5: Node 12/npm 6
- plotly.js from 2.5: Node 16/npm 8
- plotly.js from 2.35: Node 18/npm 10

@gvwilson gvwilson added P1 needs immediate attention infrastructure build process etc. fix fixes something broken labels Aug 14, 2024
@birkskyum
Copy link
Contributor Author

birkskyum commented Aug 18, 2024

Rebased, after the addition of biome.

Copy link
Contributor

@archmoj archmoj left a comment

Choose a reason for hiding this comment

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

💃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix fixes something broken infrastructure build process etc. P1 needs immediate attention
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants