Require Node.js 12.20 and move to ESM #29
Conversation
BREAKING CHANGE: require Node.js >= v12.20
antongolub
force-pushed
the
move-to-esm
branch
2 times, most recently
from
0e0e4f7
to
74e21de
Compare
cli.js
Outdated
| }, {})); | ||
| }; | ||
| const normalizePluginOptions = plugin => | ||
| // eslint-disable-next-line unicorn/no-array-reduce |
There was a problem hiding this comment.
Would be great if you could fix the lint instead of just ignoring it.
There was a problem hiding this comment.
I can replace it with any other type of loop, but imo reduce is exactly what we need here — converting an array into a map.
There was a problem hiding this comment.
I would prefer a for-of loop. I generally, find that to be more readable.
readme.md
Outdated
| $ imagemin foo.png --plugin.webp.quality=95 --plugin.webp.preset=icon > foo-icon.webp | ||
| ``` | ||
|
|
||
| **macOS** users can also use the short CLI syntax for array arguments: |
There was a problem hiding this comment.
I think non-Windows would be more correct than macOS
There was a problem hiding this comment.
Just a typo. It should be macOS/linux. But ok, non-Windows is also fine.
cli.js
Outdated
| import stripIndent from 'strip-indent'; | ||
| import pairs from 'lodash.pairs'; | ||
|
|
||
| const require = createRequire(import.meta.url); |
There was a problem hiding this comment.
require should be replaced by await import(…).
There was a problem hiding this comment.
require('name') searches for the module up the directory tree by diving into everynode_modules/<name>. Аnd then it will also check the name existence as global package. Dynamic import checks just a single path.
There was a problem hiding this comment.
Dynamic import checks just a single path.
Are you sure about that? According to https://nodejs.org/api/esm.html#esm_resolver_algorithm_specification, static import statement searches up, so I would have assumed dynamic import does the same.
There was a problem hiding this comment.
Wow! My experience bases on early polyfills. It's great that it just works now.
cli.js
Outdated
| }, {})); | ||
| }; | ||
| const normalizePluginOptions = plugin => | ||
| // eslint-disable-next-line unicorn/no-array-reduce |
There was a problem hiding this comment.
I would prefer a for-of loop. I generally, find that to be more readable.
cli.js
Outdated
| import stripIndent from 'strip-indent'; | ||
| import pairs from 'lodash.pairs'; | ||
|
|
||
| const require = createRequire(import.meta.url); |
There was a problem hiding this comment.
Dynamic import checks just a single path.
Are you sure about that? According to https://nodejs.org/api/esm.html#esm_resolver_algorithm_specification, static import statement searches up, so I would have assumed dynamic import does the same.
|
Thanks :) |
Co-authored-by: Sindre Sorhus <sindresorhus@gmail.com>
meowupdatecloses docs(readme): add a note on how to pass arguments in [0,1] range #28
BREAKING CHANGE: require Node.js >= 12.20