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

Support ESM #746

Closed
Tracked by #2864
RyanZim opened this issue Jan 31, 2020 · 29 comments · Fixed by #974
Closed
Tracked by #2864

Support ESM #746

RyanZim opened this issue Jan 31, 2020 · 29 comments · Fixed by #974

Comments

@RyanZim
Copy link
Collaborator

RyanZim commented Jan 31, 2020

Node v13.2.0+ has experimental ESM (native module) support without any kind of flag. This means that you can now do:

import { readFile } from 'fs'

However, if you try:

import { readFile } from 'fs-extra'

You get an error, because fs-extra is CJS-only. (You cannot export named exports from a CJS file) If we are going to remain a drop-in replacement for fs, we need to support this.

@RyanZim RyanZim added this to the 9.0.0 milestone Jan 31, 2020
@RyanZim
Copy link
Collaborator Author

RyanZim commented Feb 13, 2020

Discussed with @jprichardson and agreed to postpone until ESM support is no longer consider experimental in Node; due to the challenges of implementing this.

@ChocolateLoverRaj

This comment has been minimized.

@ChocolateLoverRaj

This comment has been minimized.

@ChocolateLoverRaj

This comment has been minimized.

@RyanZim

This comment has been minimized.

@beorn
Copy link

beorn commented Nov 1, 2020

See also https://www.skypack.dev/view/fs-extra for hints/ideas.

(Of all the packages I have in my app, fs-extra was the only one that didn't work with snowpack dev due to what I presume is rollup/plugin-commonjs not supporting the rather dynamic require/import/export syntax.)

c-vetter added a commit to c-vetter/node-fs-extra that referenced this issue Dec 18, 2020
@RyanZim
Copy link
Collaborator Author

RyanZim commented Jan 18, 2021

Node v15.3.0 makes ESM stable, so we should get on with this.

atao60 added a commit to atao60/fse-cli that referenced this issue Jun 29, 2021
Migrate to ESM to allow updating packages with no more CJS support such as supports-color or
terminal-link. Don't wait for jprichardson/node-fs-extra#746.

BREAKING CHANGE: remove support to Node 10
@LukeSheard
Copy link

Is there any idea of how to tackle this? A lot of other nom packages are quickly dropping commonjs support and it would be great to support native ESM syntax on fs-extra.

@RyanZim
Copy link
Collaborator Author

RyanZim commented Jul 22, 2021

There's a few holdups:

  1. We depend on graceful-fs, which is not ESM
  2. Handling of functions that are not in all versions of node
  3. Our promise polyfill behavior

See #854 for more detailed discussion.

@LukeSheard
Copy link

  1. It's still possible to import commonjs modules in ESM using import gracefulFS from 'graceful-fs which will fallback to whatever module.exports is, so I'm not sure this is an issue.
  2. I saw some comments about this on the PR - but would ultimately be for you to decide as the owner of the repo. My own opinion is that we don't currently get static exports defined by platform with commonjs - I'm not sure this makes it a blocker for enabling ESM package format.
  3. I didn't see anything about this - although I may have overlooked it. How is this related to ESM builds.

I'd be happy to work on this directly - let me know if there's anything I can do.

@RyanZim
Copy link
Collaborator Author

RyanZim commented Jul 22, 2021

All of these concerns basically tie back to doing named exports when we don't know the function list ahead of time. Ideally we'd just do export * from 'fs', but we can't, because 1) we use graceful-fs, which requires importing as an object; 3) we add promise support to all functions. And because of 2), we can't just hard-code a static list of functions.

@LukeSheard
Copy link

While I agree that it would be great to have static exports per platform - it's currently not how the module behaves with commonjs. The only way would be to maintain a platform dependent version of each module and link to it per platform either ahead of time of time or on installation.

@aminya
Copy link

aminya commented Jul 24, 2021

The dependency on the graceful-fs is also important here.

Currently, the bug fixed in this PR prevents us from bundling fs-extra using Parcel.
isaacs/node-graceful-fs#206

@LukeSheard
Copy link

@RyanZim Potentially the best way to do this then is to generate a file per platform and then add a test case to ensure that all the exposed functions from fs are also exposed from fs-extra? While this isn't necessary the nicest possible solution it would guarantee static imports like you desire?

@RyanZim
Copy link
Collaborator Author

RyanZim commented Jul 31, 2021

Potentially the best way to do this then is to generate a file per platform and then add a test case to ensure that all the exposed functions from fs are also exposed from fs-extra? While this isn't necessary the nicest possible solution it would guarantee static imports like you desire?

The problem is choosing which generated file to use; AFAIK, there's no way we can do that dynamically.

@LukeSheard
Copy link

Yeah - there's no way to do dynamic binding of static exports. So either there's a postinstall step to select the right file at install time or export everything but leave variables undefined (or fill them with a function which returns a platform error)

@Jaid
Copy link

Jaid commented Sep 20, 2021

I was stuck with this issue and played around with possible solutions. This compatibility bridge seems to be fine for now.

Before

🗎 test/test.js

import {mkdirp, writeFile, readJson} from "fs-extra"

After

🗎 test/test.js

import {mkdirp, writeFile, readJson} from "../src/lib/esm/fs-extra.js"

🗎 src/lib/esm/fs-extra.js

import {createRequire} from "node:module"

const require = createRequire(import.meta.url)
const commonJsModule = require("fs-extra")

export const {mkdirp} = commonJsModule
export const {writeFile} = commonJsModule
export const {readJson} = commonJsModule

@RyanZim
Copy link
Collaborator Author

RyanZim commented Sep 20, 2021

@Jaid Your bridge doesn't solve any of the issues we're dealing with, since it requires explicitly listing all functions, which we can't do.

@RyanZim
Copy link
Collaborator Author

RyanZim commented Sep 20, 2021

@jprichardson @manidlou @JPeer264 I've been doing some thinking about this issue, and I've come up with a compromise proposal that would allow us to ship basic ESM support now, while still not barring us from "doing it right" in a future release.

Our problems basically all center around the fact that we're trying to export named exports for native fs functions, which can't be done except via export * from 'fs', which we can't do, because we're not shipping vanilla fs. There's no actual issues with shipping named exports for our own fs-extra-specific functions. My proposal is simple: don't ship named exports for native fs functions. We can provide this limited ESM support via fs-extra/esm so it's explicit that /esm is not a drop-in replacement for fs (we already don't support fs/promises, so there's precedent for our drop-in support not extending to subpaths).

What does this look like practically? CJS (require('fs-extra')) works exactly how it works today. importing 'fs-extra' works exactly how it works today:

import fs from 'fs-extra' // Works, just like it does today
import { copy } from 'fs-extra' // Doesn't work, just like today

However, there's a new fs-extra/esm module, that provides the magic:

require('fs-extra/esm')
// Doesn't work, can't require an ESM module

import fs from 'fs-extra/esm'
// Works, just like `import fs from 'fs-extra'` does today

import { copy } from 'fs-extra/esm'
// Works! This is the new feature

import { writeFile, writeFileSync } from 'fs-extra/esm' 
// Doesn't work, since we don't export named exports of native fs functions

// If you're explicitly writing out the name of each function you're using via named imports,
// you will have to explicitly import what you want from the correct source, like so:
import { writeFile } from 'fs/promises'
import { writeFileSync } from 'fs'
import { copy } from 'fs-extra'
// This works

Pros:

  1. We get basic ESM support shipped
  2. We don't break the expectation of fs-extra itself being at parity with fs any more than it's already broken today.

Cons:

  1. Users have to import named fs functions directly from fs or fs/promises. This is somewhat painful, but acceptable, as fs/promises forces you to do the same thing.

Let me know your thoughts, or if I've missed anything in my plan here.

@Jaid
Copy link

Jaid commented Sep 20, 2021

@Jaid Your bridge doesn't solve any of the issues we're dealing with, since it requires explicitly listing all functions, which we can't do.

@RyanZim Sorry, I wasn't clear enough about my post. This was not intended to be a solution for supporting ESM in fs-extra. This is a solution for people who already migrated their own project to ESM and want to import fs-extra v10.0.0.

@RyanZim RyanZim added this to the 11.0.0 milestone Sep 21, 2021
@beorn
Copy link

beorn commented Feb 3, 2022

Would be nice to see this implemented.

@bd82
Copy link

bd82 commented Aug 25, 2022

I think that in 6+ months or so all maintained nodejs versions (16+) will support ESM natively.

@rxliuli
Copy link

rxliuli commented Sep 29, 2022

I finally decided to maintain a copy of this module myself in monorepo, automatically generate the above adaptation code according to fs-extra, and share it here

import fsExtra from 'fs-extra'
import { builders as b, namedTypes as n } from 'ast-types'
import { prettyPrint } from 'recast'
import tsParser from 'recast/parsers/typescript'
import path from 'path'
import { fileURLToPath } from 'url'
import { difference } from 'lodash-es'

function scan() {
  const excludes = [
    'FileReadStream',
    'FileWriteStream',
    '_toUnixTimestamp',
    'F_OK',
    'R_OK',
    'W_OK',
    'X_OK',
    'gracefulify',
  ]
  return difference(Object.keys(fsExtra), excludes)
}

function generate(list: string[]) {
  return b.program([
    b.importDeclaration([b.importDefaultSpecifier(b.identifier('fsExtra'))], b.literal('fs-extra')),
    ...list.map((s) =>
      b.exportNamedDeclaration(
        b.variableDeclaration('const', [
          b.variableDeclarator(b.identifier(s), b.memberExpression(b.identifier('fsExtra'), b.identifier(s))),
        ]),
      ),
    ),
  ])
}

export async function build() {
  const list = scan()
  const ast = generate(list)
  const code = prettyPrint(ast, { parser: tsParser }).code
  await fsExtra.writeFile(path.resolve(path.dirname(fileURLToPath(import.meta.url)), 'index.ts'), code)
}

Output

import fsExtra from 'fs-extra'
export const appendFile = fsExtra.appendFile
export const appendFileSync = fsExtra.appendFileSync
export const access = fsExtra.access
export const accessSync = fsExtra.accessSync
export const chown = fsExtra.chown
export const chownSync = fsExtra.chownSync
export const chmod = fsExtra.chmod
export const chmodSync = fsExtra.chmodSync
export const close = fsExtra.close
export const closeSync = fsExtra.closeSync
export const copyFile = fsExtra.copyFile
export const copyFileSync = fsExtra.copyFileSync
export const cp = fsExtra.cp
export const cpSync = fsExtra.cpSync
export const createReadStream = fsExtra.createReadStream
export const createWriteStream = fsExtra.createWriteStream
export const exists = fsExtra.exists
export const existsSync = fsExtra.existsSync
export const fchown = fsExtra.fchown
export const fchownSync = fsExtra.fchownSync
export const fchmod = fsExtra.fchmod
export const fchmodSync = fsExtra.fchmodSync
export const fdatasync = fsExtra.fdatasync
export const fdatasyncSync = fsExtra.fdatasyncSync
export const fstat = fsExtra.fstat
export const fstatSync = fsExtra.fstatSync
export const fsync = fsExtra.fsync
export const fsyncSync = fsExtra.fsyncSync
export const ftruncate = fsExtra.ftruncate
export const ftruncateSync = fsExtra.ftruncateSync
export const futimes = fsExtra.futimes
export const futimesSync = fsExtra.futimesSync
export const lchown = fsExtra.lchown
export const lchownSync = fsExtra.lchownSync
export const lchmod = fsExtra.lchmod
export const lchmodSync = fsExtra.lchmodSync
export const link = fsExtra.link
export const linkSync = fsExtra.linkSync
export const lstat = fsExtra.lstat
export const lstatSync = fsExtra.lstatSync
export const lutimes = fsExtra.lutimes
export const lutimesSync = fsExtra.lutimesSync
export const mkdir = fsExtra.mkdir
export const mkdirSync = fsExtra.mkdirSync
export const mkdtemp = fsExtra.mkdtemp
export const mkdtempSync = fsExtra.mkdtempSync
export const open = fsExtra.open
export const openSync = fsExtra.openSync
export const opendir = fsExtra.opendir
export const opendirSync = fsExtra.opendirSync
export const readdir = fsExtra.readdir
export const readdirSync = fsExtra.readdirSync
export const read = fsExtra.read
export const readSync = fsExtra.readSync
export const readv = fsExtra.readv
export const readvSync = fsExtra.readvSync
export const readFile = fsExtra.readFile
export const readFileSync = fsExtra.readFileSync
export const readlink = fsExtra.readlink
export const readlinkSync = fsExtra.readlinkSync
export const realpath = fsExtra.realpath
export const realpathSync = fsExtra.realpathSync
export const rename = fsExtra.rename
export const renameSync = fsExtra.renameSync
export const rm = fsExtra.rm
export const rmSync = fsExtra.rmSync
export const rmdir = fsExtra.rmdir
export const rmdirSync = fsExtra.rmdirSync
export const stat = fsExtra.stat
export const statSync = fsExtra.statSync
export const symlink = fsExtra.symlink
export const symlinkSync = fsExtra.symlinkSync
export const truncate = fsExtra.truncate
export const truncateSync = fsExtra.truncateSync
export const unwatchFile = fsExtra.unwatchFile
export const unlink = fsExtra.unlink
export const unlinkSync = fsExtra.unlinkSync
export const utimes = fsExtra.utimes
export const utimesSync = fsExtra.utimesSync
export const watch = fsExtra.watch
export const watchFile = fsExtra.watchFile
export const writeFile = fsExtra.writeFile
export const writeFileSync = fsExtra.writeFileSync
export const write = fsExtra.write
export const writeSync = fsExtra.writeSync
export const writev = fsExtra.writev
export const writevSync = fsExtra.writevSync
export const Dir = fsExtra.Dir
export const Dirent = fsExtra.Dirent
export const Stats = fsExtra.Stats
export const ReadStream = fsExtra.ReadStream
export const WriteStream = fsExtra.WriteStream
export const constants = fsExtra.constants
export const promises = fsExtra.promises
export const copy = fsExtra.copy
export const copySync = fsExtra.copySync
export const emptyDirSync = fsExtra.emptyDirSync
export const emptydirSync = fsExtra.emptydirSync
export const emptyDir = fsExtra.emptyDir
export const emptydir = fsExtra.emptydir
export const createFile = fsExtra.createFile
export const createFileSync = fsExtra.createFileSync
export const ensureFile = fsExtra.ensureFile
export const ensureFileSync = fsExtra.ensureFileSync
export const createLink = fsExtra.createLink
export const createLinkSync = fsExtra.createLinkSync
export const ensureLink = fsExtra.ensureLink
export const ensureLinkSync = fsExtra.ensureLinkSync
export const createSymlink = fsExtra.createSymlink
export const createSymlinkSync = fsExtra.createSymlinkSync
export const ensureSymlink = fsExtra.ensureSymlink
export const ensureSymlinkSync = fsExtra.ensureSymlinkSync
export const readJson = fsExtra.readJson
export const readJsonSync = fsExtra.readJsonSync
export const writeJson = fsExtra.writeJson
export const writeJsonSync = fsExtra.writeJsonSync
export const outputJson = fsExtra.outputJson
export const outputJsonSync = fsExtra.outputJsonSync
export const outputJSON = fsExtra.outputJSON
export const outputJSONSync = fsExtra.outputJSONSync
export const writeJSON = fsExtra.writeJSON
export const writeJSONSync = fsExtra.writeJSONSync
export const readJSON = fsExtra.readJSON
export const readJSONSync = fsExtra.readJSONSync
export const mkdirs = fsExtra.mkdirs
export const mkdirsSync = fsExtra.mkdirsSync
export const mkdirp = fsExtra.mkdirp
export const mkdirpSync = fsExtra.mkdirpSync
export const ensureDir = fsExtra.ensureDir
export const ensureDirSync = fsExtra.ensureDirSync
export const move = fsExtra.move
export const moveSync = fsExtra.moveSync
export const outputFile = fsExtra.outputFile
export const outputFileSync = fsExtra.outputFileSync
export const pathExists = fsExtra.pathExists
export const pathExistsSync = fsExtra.pathExistsSync
export const remove = fsExtra.remove
export const removeSync = fsExtra.removeSync

Using

import { readFile } from '@liuli-util/fs-extra'

console.log(await readFile(import.meta.url))

Hopefully after fs-extra supports esm I can simply switch back

@dscalzi
Copy link

dscalzi commented Oct 5, 2022

Any movement on this? Some libs are starting to enforce hard dependencies on ESM so it's becoming challenging to get everything to play nice together.

@rosskevin
Copy link

Unfortunately, I'm abandoning fs-extra now because we have switched to ESM only. It only showed up at runtime...TypeError: o.readFileSync is not a function

It would be great if there were an ESM bundle...

@RyanZim
Copy link
Collaborator Author

RyanZim commented Oct 25, 2022

Implementation in #974, slated for release in v11; please review.

RyanZim added a commit that referenced this issue Nov 28, 2022
* Remove process.cwd() trick from test files

* BREAKING: Switch from main to exports

* Add fs-extra/esm ESM named import module, with just fs-extra methods

Fixes #746
@rxliuli
Copy link

rxliuli commented Dec 10, 2022

Not really solved, it doesn't work whether I use import { readdir } from 'fs-extra' or import { readdir } from 'fs-extra/esm'

ref: https://github.com/rxliuli/fs-extra-demo

@RyanZim
Copy link
Collaborator Author

RyanZim commented Dec 10, 2022

readdir is a native fs function; as per https://github.com/jprichardson/node-fs-extra#esm, you need to import these functions directly from fs or fs/promises for named imports.

@dscalzi
Copy link

dscalzi commented Dec 11, 2022

Types still need to be updated https://github.com/DefinitelyTyped/DefinitelyTyped/tree/master/types/fs-extra. Right now when using typescript, the esm export is basically an any, so you won't know which imports are wrong until runtime.

Posted this discussion but it seems to not be getting any traction. DefinitelyTyped/DefinitelyTyped#63476

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants