paulmillr / readdirp

Recursive version of fs.readdir with streaming api.
https://paulmillr.com
MIT License
378 stars 51 forks source link

ESM module #163

Open talentlessguy opened 4 years ago

talentlessguy commented 4 years ago

Hello, thanks for making this module! I'm the author of tinyhttp framework and it has @tinyhttp/static middleware which uses readdirp. As I noticed, it doesn't have a "module" field.

Is it a good idea to add this field to package.json so it will work? Or better not touch it?

paulmillr commented 4 years ago

Hi. It prob won’t work.

talentlessguy commented 4 years ago

@paulmillr hi, I made it work in other Node.js packages, here's one of them: https://github.com/talentlessguy/es-accepts

This is how it would look like:

// package.json
 "main": "index.cjs",
  "module": "index.js",
  "types": "index.d.ts",
"type": "module",
  "exports": {
    ".": {
      "import": "./index.js",
      "require": "./index.cjs"
    },
    "./package.json": "./package.json",
    "./": "./"
  },

I can submit a pull request with adding the required fields and an ESM file

paulmillr commented 4 years ago

What is index.cjs, how does it look like?

What is the benefit of switching to cjs ?

talentlessguy commented 4 years ago

@paulmillr .cjs is basically the same as .js with require, but when you use modules, Node.is will recommend to use .js for ESM and .cjs for CommonJS

Another way is using .mjs for ESM and .js for CommonJS, and it will still work

tldr; cjs = js w/ require

MVEMCJSUNPE commented 3 years ago

Here are some relevant articles that explain what @talentlessguy is talking about: packages determining module system hybrid npm packages package checks

calumk commented 3 years ago

@talentlessguy - Is this fixed or no longer needed, or some other solution?

installing / running @tinyhttp/static provides the error:

SyntaxError: Named export 'promise' not found. The requested module 'readdirp' is a CommonJS module, which may not support all module.exports as named exports.

I know its probably an error with @tinyhttp/static not with this repo, but since the discussion for it seems to be here, thought i would continue this thread.

talentlessguy commented 3 years ago

@calumk @tinyhttp/static has been deleted from the repo... probably I should also add a deprecation warning

use sirv instead