dimsemenov / PhotoSwipe

JavaScript image gallery for mobile and desktop, modular, framework independent
http://photoswipe.com
MIT License
24.18k stars 3.31k forks source link

TypeScript and v5 #1983

Closed philiplindberg closed 1 year ago

philiplindberg commented 1 year ago

I just upgraded to v5 in my react typescript project, and following the getting started guide in the docs gave me the following typescript error:

Could not find a declaration file for module 'photoswipe/dist/photoswipe-lightbox.esm.js'

when importing the library as follows:

import PhotoSwipeLightbox from 'photoswipe/dist/photoswipe-lightbox.esm.js'

I also tried importing the library as shown in the for React guide (import PhotoSwipeLightbox from 'photoswipe/lightbox'), but got the same error. Here's a repro demonstrating the issue:

git clone https://github.com/philiplindberg/photoswipe-repro
cd photoswipe-repro
yarn
code .
# open in VS Code and notice typescript error in `src/App.tsx`

I was finally able to make typescript happy by specifying the following path in my tsconfig.json:

{
  "compilerOptions": {
    "paths": {
      "photoswipe/lightbox": ["./node_modules/photoswipe/dist/types/lightbox/lightbox.d.ts"]
    }
  }
}

While this works, it seems like a less than ideal workaround. Is there a better way to import typescript types for the library?

au5ton commented 1 year ago

How to fix this issue (for consumers of PhotoSwipe)

Setting tsconfig.json > moduleResolution to node16 fixes the "Cannot find module 'photoswipe/lightbox' or its corresponding type declarations." error for me.

Correcting the issue (for PhotoSwipe maintainers)

Some extra clarification in the documentation to specify that TypeScript consumers need to use moduleResolution set to node16 or nodenext could address the issue. Alternatively, the build output could also use real folders that contain copies of the files (similar to the examples below) so that even legacy consumers can use the library, but that's up to the maintainers to decide.

Why the issue happens

I think this issue has to do with a TypeScript configuration issue in people who consume this library. TypeScript added support for Node-based ESM resolving in TypeScript 4.7 -- this includes entries in package.json like so:

https://github.com/dimsemenov/PhotoSwipe/blob/05e2193bf94b9bbabab36a288f8788e32c55adf9/package.json#L8-L19

However, support for this is will only reflect in TypeScript if tsconfig.json > moduleResolution is set to node16 or nodenext. Without this, TypeScript will ignore entries in package.json > exports and an import like 'photoswipe/lightbox' (when the folder '/lightbox' doesn't exist in the build output) will fail.

Some packages (such as swr and @mui/material for example) opt to circumvent this compatibility requirement by creating real folders in their build output so they don't have to rely on Node.js 16's module resolution to map to the correct folder.

Sources:

Related issues

1881 #1937

@dimsemenov

dimsemenov commented 1 year ago

@au5ton thanks for the clarification, I'll add this to the documentation somewhere. Adding extra folders at this point would mess up the repo structure since naming is not too consistent. So I'm not sure if it's a good idea...

kimgiutae commented 1 year ago

Hi @dimsemenov and @au5ton.

I'm running a react project with next.js v13 with typescript v5. And even with your suggestions the import still cause a typescript error.

If compilerOptions.moduleResolution is "node" the ts error is: "Cannot find module 'photoswipe/lightbox' or its corresponding type declarations.ts(2307)"

Is compilerOptions.moduleResolution is "node16" or "nodenext" the ts error is: "The current file is a CommonJS module whose imports will produce 'require' calls; however, the referenced file is an ECMAScript module and cannot be imported with 'require'. Consider writing a dynamic 'import("photoswipe/lightbox")' call instead. To convert this file to an ECMAScript module, add the field "type": "module" to 'd:/project/package.json'.ts(1479)"

I mean... the code works and I even get types autocompletion, but i dont know the origin of the error. I supose that is related to some kind of incompatibility between compilation sources.