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

fix: add package main field for TypeScript support #981

Merged
merged 2 commits into from Nov 29, 2022
Merged

fix: add package main field for TypeScript support #981

merged 2 commits into from Nov 29, 2022

Conversation

mtdvlpr
Copy link
Contributor

@mtdvlpr mtdvlpr commented Nov 29, 2022

Fixes #979

@mtdvlpr mtdvlpr changed the title fix: add legacy main and module for TypeScript support fix: add legacy main/module for TypeScript support Nov 29, 2022
Copy link
Contributor

@caugner caugner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See also: #980

Tested locally, and resolves the issue with TypeScript.

@caugner
Copy link
Contributor

caugner commented Nov 29, 2022

It doesn't resolve the issue with import ... from "fs-extra/esm" though. 🤔

@mtdvlpr
Copy link
Contributor Author

mtdvlpr commented Nov 29, 2022

It doesn't have to, right? Importing from fs-extra will get the esm version if I'm not mistaken.

@caugner
Copy link
Contributor

caugner commented Nov 29, 2022

It doesn't have to, right? Importing from fs-extra will get the esm version if I'm not mistaken.

If I understand @RyanZim's intention correctly, the separate fs-extra/esm export aims to avoid confusion. Adding the "module" field reintroduces that confusion, because you could then import from fs-extra and get the ESM module.

We could just add the main field for now, and that alone would resolve TypeScript issue.

Copy link
Contributor

@caugner caugner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A more conservative solution is to omit the module field for now, because the author does not intend to export the ESM module at fs-extra.

package.json Outdated Show resolved Hide resolved
Co-authored-by: Claas Augner <495429+caugner@users.noreply.github.com>
@mtdvlpr mtdvlpr changed the title fix: add legacy main/module for TypeScript support fix: add legacy main for TypeScript support Nov 29, 2022
Copy link
Contributor

@caugner caugner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-tested and still resolves the issue when using import fs from "fs-extra"; in TypeScript.

@mtdvlpr mtdvlpr changed the title fix: add legacy main for TypeScript support fix: add main field for TypeScript support Nov 29, 2022
@mtdvlpr mtdvlpr changed the title fix: add main field for TypeScript support fix: add package main field for TypeScript support Nov 29, 2022
@RyanZim
Copy link
Collaborator

RyanZim commented Nov 29, 2022

I removed the main field, since it wasn't needed for backward compatibility with old Node versions, as we don't support Node versions old enough not to support exports. However, since it seems TS doesn't like this (potentially a bug? see #979 (comment)), it seems wise to add it back for now.

It doesn't resolve the issue with import ... from "fs-extra/esm" though.

I think that's fine; main is only needed for TS support, and if you're using TS, you already have named import support; there's no reason to use fs-extra/esm in TS.

@caugner is correct; we don't want to add module here, as that would be confusing.

@RyanZim RyanZim merged commit ab86a8a into jprichardson:master Nov 29, 2022
@RyanZim
Copy link
Collaborator

RyanZim commented Nov 29, 2022

Published in v11.1.0 🎉

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.

Cannot find module 'fs-extra/esm' or its corresponding type declarations.ts(2307)
3 participants