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

move to minimum node 12; remove rimraf #237

Merged
merged 2 commits into from
Apr 15, 2021
Merged

move to minimum node 12; remove rimraf #237

merged 2 commits into from
Apr 15, 2021

Conversation

brendankenny
Copy link
Member

Node 10 will be EoL on April 30, just about the time we might possibly do another release of chrome-launcher, so seems ok to do now.

We should bump the next release to 0.14.0, though.

With node 12 comes fs.rmdir(path, {recursive: true}), so we don't need rimraf anymore.

@@ -1,5 +1,4 @@
# Chrome Launcher [![Linux Build Status](https://img.shields.io/travis/GoogleChrome/chrome-launcher/master.svg)](https://travis-ci.org/GoogleChrome/chrome-launcher) [![Windows Build Status](https://img.shields.io/appveyor/ci/paulirish/chrome-launcher/master.svg)](https://ci.appveyor.com/project/paulirish/chrome-launcher/branch/master) [![NPM chrome-launcher package](https://img.shields.io/npm/v/chrome-launcher.svg)](https://npmjs.org/package/chrome-launcher)

# Chrome Launcher [![GitHub Actions Status Badge](https://github.com/GoogleChrome/chrome-launcher/workflows/🛠/badge.svg)](https://github.com/GoogleChrome/chrome-launcher/actions) [![NPM chrome-launcher package](https://img.shields.io/npm/v/chrome-launcher.svg)](https://npmjs.org/package/chrome-launcher)
Copy link
Member Author

Choose a reason for hiding this comment

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

drive-by badge fix for the migration to github actions

@@ -2,7 +2,7 @@
"compilerOptions": {
"module": "commonjs",
"outDir": "dist",
"target": "es2016",
"target": "es2019",
Copy link
Member Author

@brendankenny brendankenny Apr 14, 2021

Choose a reason for hiding this comment

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

as officially suggested here. Savings in dist/ are pretty small (about 3KiB), but it's nice that async/await is no longer transpiled, yielding a pretty big readability improvement in chrome-launcher.js if you aren't using source maps.

@lukebro
Copy link
Contributor

lukebro commented Jun 11, 2021

In node v14.14+ a deprecation warning is now always given:

[DEP0147] DeprecationWarning: In future versions of Node.js, fs.rmdir(path, { recursive: true }) will be removed. Use fs.rm(path, { recursive: true }) instead
(Use `node --trace-deprecation ...` to show where the warning was created)

https://nodejs.org/api/deprecations.html#DEP0147

In v16+ node ignores the recursive flag. I can submit a PR for a feature check if fs.rm is available to use that instead of fs.rmdir.

let rm = fs.rm || fs.rmdir;

rm(path, { recursive: true });

That way we can stay backwards compatible with v12.

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