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

refactor: remove fs-extra dependency #206

Closed
wants to merge 2 commits into from
Closed

Conversation

ckerr
Copy link
Member

@ckerr ckerr commented Jun 28, 2024

Bump Node.js to 14.14.0 to pick up fs.rm(), use it, and drop the fs-extra dependency.

@ckerr ckerr added dependencies Pull requests that update a dependency file javascript Pull requests that update Javascript code labels Jun 28, 2024
@ckerr ckerr requested a review from a team as a code owner June 28, 2024 22:08
Copy link
Member

@dsanders11 dsanders11 left a comment

Choose a reason for hiding this comment

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

This would need to be marked as a breaking change (BREAKING CHANGE: in the commit body).

@electron/wg-ecosystem policy has generally been to avoid Node.js version bumps unless necessary to avoid breaking changes on packages which fragment the install base.

I'd want to look around to see if there are any other breaking changes we can make, like more coe removal as follow-up to #187, etc, if we were going to commit to the breaking change here.

Also, do we want to do a bigger jump, to Node.js 16? I think we need to look at the general ecosystem and see how much we can lift and shift with regards to Node.js version.

@ckerr
Copy link
Member Author

ckerr commented Jun 28, 2024

If we're going to move the ecosystem's minimum Node.js version as a whole, v16.7.0 would be nice-to-have to pick up fs.cp()

@ckerr ckerr force-pushed the refactor/remove-fs-extra branch from 451dbc8 to 6134d7b Compare June 28, 2024 22:26
@ckerr
Copy link
Member Author

ckerr commented Jun 28, 2024

@dsanders11 I've pushed up a rebase reword that includes the BREAKING CHANGE text.

+1 on deferring this pending discussion on minimum Node.js version in the ecosystem

@MarshallOfSound
Copy link
Member

I've made this case before and will make it again in Ecosystem meetings. Until there is a good forcing function to push us to a higher node version I see only harm can come from bumping the min node version and fragmenting the ecosystem across major versions.

Removing a dependency as ubiquitous as fs-extra isn't a solid reason IMO, because we use it in everything (and lots of other folks do to) it's only installed once normally anyway 🤷 I believe this is on the docket for summit to discuss anyway

@ckerr ckerr closed this Jun 29, 2024
@ckerr ckerr deleted the refactor/remove-fs-extra branch June 29, 2024 18:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file javascript Pull requests that update Javascript code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants