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

feat: support for esm named imports #986

Closed
wants to merge 1 commit into from
Closed

feat: support for esm named imports #986

wants to merge 1 commit into from

Conversation

rxliuli
Copy link

@rxliuli rxliuli commented Dec 10, 2022

motivation

There are still problems with esm support at present, and named imports are not really available. For example, the following code will report an error when using the latest version of fs-extra

import { readdir } from 'fs-extra'
import fsExtra from 'fs-extra'
import { fileURLToPath } from 'url'
import path from 'path'

const __filename = fileURLToPath(import.meta.url)
const __dirname = path.dirname(__filename)
console.log(await readdir(__dirname))
console.log(await fsExtra.readdir(__dirname))

This PR addresses the following issues

  • Named imports and default imports support in esm
    import { readdir } from 'fs-extra'
    import fsExtra from 'fs-extra'
    import { fileURLToPath } from 'url'
    import path from 'path'
    
    const __filename = fileURLToPath(import.meta.url)
    const __dirname = path.dirname(__filename)
    console.log(await readdir(__dirname))
    console.log(await fsExtra.readdir(__dirname))
  • named import and default import support in cjs
    import { readdir } from 'fs-extra'
    import fsExtra from 'fs-extra'
    const { readdir: readdirCjs } = require('fs-extra')
    const fsExtraCjs = require('fs-extra')
    ;(async () => {
      console.log(await readdir(__dirname))
      console.log(await readdirCjs(__dirname))
      console.log(await fsExtra.readdir(__dirname))
      console.log(await fsExtraCjs.readdir(__dirname))
    })()
  • Properly support the use in ts, no longer use fs-extra/esm

For specific test methods, see: https://github.com/rxliuli/fs-extra-demo

@RyanZim
Copy link
Collaborator

RyanZim commented Dec 10, 2022

This approach doesn't work for a couple of reasons:

  • fs.lchown is not available on some Linuxes
  • This doesn't allow us to export functions that are only available in some Node.js versions (not the case now, but has been in the past and likely will be in the future).

For more details, see discussion on #746 & #854

@RyanZim RyanZim closed this Dec 10, 2022
@rxliuli
Copy link
Author

rxliuli commented Dec 11, 2022

This approach doesn't work for a couple of reasons:

  • fs.lchown is not available on some Linuxes
  • This doesn't allow us to export functions that are only available in some Node.js versions (not the case now, but has been in the past and likely will be in the future).

For more details, see discussion on #746 & #854

What I don't understand is why the export interface of its cjs version is different from that of esm version. In cjs, all the methods of fs itself can be used, but in esm export, it is not included.
I understand that fs may have different exports between nodejs versions, while cjs always forwards and exposes all, and esm must be explicitly specified, but this can be solved by version, for example, different nodejs versions are required for different major versions.
Speaking of ts support, if you do use another export point fs-extra/esm, even if the types are fixed, this can easily be mis-imported, because you have to take care to avoid importing from fs-extra, which can be annoying

@RyanZim
Copy link
Collaborator

RyanZim commented Dec 12, 2022

this can be solved by version, for example, different nodejs versions are required for different major versions.

It's not practical to manage multiple major version release lines for different Node versions. This also still doesn't solve the obstacle with fs.lchown.

@lvjiaxuan
Copy link

Feel sad for this.

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

Successfully merging this pull request may close these issues.

None yet

3 participants