sindresorhus / globby

User-friendly glob matching
MIT License
2.51k stars 130 forks source link

Require Node.js 12.20 and move to ESM #181

Closed antongolub closed 3 years ago

antongolub commented 3 years ago
sindresorhus commented 3 years ago

Thanks for working on this.

You need to update index.d.ts (https://github.com/sindresorhus/typescript-definition-style-guide) and the readme for ESM too.

antongolub commented 3 years ago

rfr

sindresorhus commented 3 years ago

I can never decide how to handle named exports with async and sync methods:

The former looks nicer, but the latter is clearer. Any opinion?

antongolub commented 3 years ago

My guess is that for most users, the migration will look like this: const globby = require('globby') → import {globbyAsync as globby} from 'globby'. Well... I'd prefer just import {globby} from 'globby' instead.

sindresorhus commented 3 years ago

Alright. Let's go with globby for async and drop globbyAsync.

antongolub commented 3 years ago

@sindresorhus, your turn

zloirock commented 3 years ago

@sindresorhus sorry, why do you avoid the default export here?

fisker commented 3 years ago

I guess for consistency import? But I think maybe we can export both named and default? export {globby as default, globby}

sindresorhus commented 3 years ago

It's weird to have one default and one named when there are two main exports. I only do default and named export mix when there's only one main export and some secondary exports (like error, or helper utilities).

sindresorhus commented 3 years ago

But I think maybe we can export both named and default? export {globby as default, globby}

No, there should be only one way to import the thing. Aliases create confusion and inconsistency.