sindresorhus / globby

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

Must use import to load ES Module #193

Closed yankeeinlondon closed 2 years ago

yankeeinlondon commented 2 years ago

I have a Typescript project which has been using globby for a long time but I just upgraded to the latest version.

I noticed that exports are all named exports now which I greatly prefer and I switched my code to using that syntax. No problem there and also zero problem using Typescript's tsc compiler to build the program. Running the program, however, immediately reports the following error:

node:internal/modules/cjs/loader:1109 throw new ERR_REQUIRE_ESM(filename, parentPath, packageJsonPath); ^

Error [ERR_REQUIRE_ESM]: Must use import to load ES Module: /Volumes/Coding/inocan/open-source/do-devops/node_modules/globby/index.js require() of ES modules is not supported. require() of /Volumes/Coding/inocan/open-source/do-devops/node_modules/globby/index.js from /Volumes/Coding/inocan/open-source/do-devops/bin/shared/file/getDataFiles.js is an ES module file as it is a .js file whose nearest parent package.json contains "type": "module" which defines all .js files in that package scope as ES modules.

My program is a CLI program and therefore much better left as commonjs, is there no way to import this successfully?

yankeeinlondon commented 2 years ago

please note I am running using Node 14 and 15

sindresorhus commented 2 years ago

Please read the release notes and the gist linked from it.

sindresorhus commented 2 years ago

My program is a CLI program and therefore much better left as commonjs

This is not correct. CLIs can be native ESM.

yankeeinlondon commented 2 years ago

sure they can be but they limit the user pool

yankeeinlondon commented 2 years ago

it's a library so I can't assume everyone's on Node 14

alexanderby commented 2 years ago

@ksnyde So what's the solution?

yankeeinlondon commented 2 years ago

I just downgraded to the older version of globby; I am using pure ES in some repo chains but there is still a lot of pain points in converting for some chains so for these I'm waiting for the tooling support and help documents to make the process easier. Can't take on migration all at once ... still only get 24 hours in a day.

ashleydavis commented 2 years ago

I tried the latest version (12.0.2) and it still has this problem.

I can confirm that downgrading to 11.0.4 fixes the problem.

ashleydavis commented 2 years ago

Globby has been my fav globbing library for many years, it's a real shame that it no longer works in Node.js 14.

RichieRunner commented 2 years ago

I tried the latest version (12.0.2) and it still has this problem.

I can confirm that downgrading to 11.0.4 fixes the problem.

Confirmed, downgrading also worked for me with NodeJS 16.x. Thanks!

mitchellsimoens commented 2 years ago

You can still use ESM within CommonJS but not in the same way. You need to use const foo = await import('something') which means you need top-level async or within an async method. For example, I changed this:

const { globbySync } = require('globby')
const files = globbySync('./content/**/*.md')

// ...

to this

(async () => {
  const { globbySync } = await import('globby')
  const files = globbySync('./content/**/*md')

  // ...
})()

Tested using globby 13.0.0 with Node 14.17.6 and Node 17.3.0 (simple script that doesn't output anything but no errors is no errors):

globby test
MisterFixx commented 1 year ago

Time to find a new library I guess, I'm not dealing with this bullshit.

pinuke commented 1 year ago

Require was better for synchronous code IMO.

ES6 focuses way too much on promisification/async code. If I'm trying to rewrite a legacy batch script into nodejs, I don't want to have to deal with ES6 import or CommonJS's dynamic import().

Promisification isn't the solution to every problem...

pinuke commented 1 year ago

This should be a pretty clear metric that v12/13 are a downgrade to v11:

Screenshot 2023-02-12 7 07 01 PM

aovchinn commented 1 year ago

as node 12 requires --experimental-modules for this, maybe it makes sense to add engines.node restriction in package.json of globby@13, to support only node >= 14 it is supported since 12.22

liudonghua123 commented 1 year ago

why not provide both esm and cjs support. It shouldn't difficult and complex.

I wrote a simple node lib which support esm and cjs recently. See https://github.com/liudonghua123/ids_api/blob/main/package.json

isaachinman commented 1 year ago

@smartguy1196 It's been on my mind recently if it would be possible/easy to create an npm package that wrapped these ESM-only packages and re-exported CJS versions. Especially for the packages that @sindresorhus maintains, I assume the build pipeline would be easy to extend.

Eg:

import { globby } from '@cjs-exporter/globby'

I'm well aware of the current situation with the JS community, but the metrics do speak for themselves, and these things have inertia.

malash commented 1 year ago

@isaachinman Great idea.

Just like @types/aaa is to non-typed package aaa as @cjs-exporter/bbb is to esm only package bbb. The @cjs-exporter/ could be a community-maintained project to add cjs support for esm packages.

In technical, this pipeline could fully automatically transform all JS code with @babel/plugin-transform-modules-commonjs and release a new version each time the upstream released.

isaachinman commented 1 year ago

I had a few minutes free, so I've set this up.

Repository is here.

The CJS version of globby@13.1.3 can now be installed as CJS via:

yarn add @cjs-exporter/globby
npm install @cjs-exporter/globby

This was a quick rough draft. There are some issues to do with bundling node modules or not – some of which may contain ESM-only modules themselves.

Anyways, if people are interested in it, I am happy to maintain the project. Please open issues on the repo linked above.

glassdimlygr commented 11 months ago

Either way I use globby 13 it barfs like this. It seems like some kind of internal mismatch between module and cjs. TBH I will just use version 11.

const {globby} = require('globby'); or import { globby } from 'globby';

Errors.

Looks like it's recommending the import('globby') async syntax. I'm not a huge fan of that. It's going to confuse my devs every time they go to use it. I think I'll just stick this in package.json:

"globby": "11.0.4"

and then just do this:

// eslint-disable-next-line @typescript-eslint/no-var-requires
const globby = require('globby');

I get that module maintainers want to push the JS community forward by supporting ESM only. But we ain't there yet. How are we gonna get there as a community? It's been ~7 years of confusion. Sigh.

Anyhow, thanks for globby, maintainers. I don't like your decision but I respect your work.

isaachinman commented 11 months ago

push the JS community forward by supporting ESM only

It was a ridiculous decision at the time and has aged even worse. The solution is probably going to end up being something like bun.sh, so we can get back to writing code and forget about all this nonsense/politics.