Fix TS issues, more correctly ship CJS and ESM

[RobbieTheWagner]

No description provided.

[joshkel]

I'm still working through all the ins and outs involved in supporting CJS + ESM across different configurations, but some initial comments:

It looks like you've set up subpath exports so users can do code like import type { Tour } from 'shepherd.js/tour', and you're using the types-versions-wildcards approach from example-subpath-export-ts-compat to implement this. Is that correct?

Do you need subpath exports? If you make everyone import as, e.g., import type { Tour } from 'shepherd.js', then I think things will be simpler. Then you'd have the option of rolling up the types into a single .d.ts file, which might address the current "Internal resolution errors" from attw.

[joshkel]

The moduleResolution: Node10 issues flagged by attw are caused by typesVersions, which is only needed to make subpath exports work with moduleResolution: Node10. tsc --traceResolution with a sample project shows that this is failing because TypeScript takes the * as a literal substitution - it tries to resolve dist/cjs/shepherd.cjs from main, substitutes it in the *, and gets dist/esm/dist/cjs/shepherd.cjs.

If you add a specific mapping for shepherd.cjs, then moduleResolution: node10 seems to work:

      "dist/cjs/shepherd.cjs": [
        "./dist/cjs/shepherd.d.cts"
      ],
      "*": [
        "./dist/cjs/*"
      ]

However, I'm not sure that you need typesVersions - see comments elsehwere.

[RobbieTheWagner]

Removing typesVersions or changing it with what you suggest here are both fine with me. I had deleted typesVersions a few times and it didn't seem to change attw at all whether I had it or not.

[RobbieTheWagner]

@joshkel

It looks like you've set up subpath exports so users can do code like import type { Tour } from 'shepherd.js/tour', and you're using the types-versions-wildcards approach from example-subpath-export-ts-compat to implement this. Is that correct?

Yeah, more or less.

Do you need subpath exports? If you make everyone import as, e.g., import type { Tour } from 'shepherd.js', then I think things will be simpler. Then you'd have the option of rolling up the types into a single .d.ts file, which might address the current "Internal resolution errors" from attw.

I'm fine with removing subpath exports. I mostly don't know what I am doing here, so a lot of this was just trying to make stuff work. One thing to note though, I opened an issue on attw and they said Svelte is not supported, so not sure if we will be able to resolve all the issues or not, but happy to try whatever you suggest.

[RobbieTheWagner]

@joshkel I used api-extractor to rollup the types, and I think things should work for everyone now 🎉

[RobbieTheWagner]

@joshkel with the newly consolidate types our dist is much simpler and attw is happy, even on node10 🎉 . However, it seems like a lot of types flat out don't work and builds blow up in the real world. You can see in CI we cannot even run a build because the react package hits a bunch of type issues like this:

packages/react build: src/index.tsx:22:14 - error TS4023: Exported variable 'useShepherd' has or is using name 'StepOptions' from external module "/home/runner/work/shepherd/shepherd/shepherd.js/dist/cjs/shepherd" but cannot be named.
packages/react build: 22 export const useShepherd = () => {

Any ideas where we may have gone wrong or how to fix this?

[RobbieTheWagner]

@joshkel I am going back to the drawing board and attempting to use rollup-plugin-ts because it is supposed to handle all this stuff for us, but I have been unable to get it to run.

[RobbieTheWagner]

@joshkel okay, so my latest attempt just uses svelte2tsx and it seems to be pretty close to working.

┌───────────────────┬──────────────────────────────┐
│                   │ "shepherd.js"                │
├───────────────────┼──────────────────────────────┤
│ node10            │ 🥴 Internal resolution error │
├───────────────────┼──────────────────────────────┤
│ node16 (from CJS) │ 🥴 Internal resolution error │
├───────────────────┼──────────────────────────────┤
│ node16 (from ESM) │ 🟢 (ESM)                     │
├───────────────────┼──────────────────────────────┤
│ bundler           │ 🟢                           │
└───────────────────┴──────────────────────────────┘

My thoughts are that CJS doesn't work because svelte 4 only supports ESM, so svelte2tsx I would imagine only generates types for ESM, therefore CJS cannot figure out what the svelte imports are. Any ideas on a way to fix things here?

[RobbieTheWagner]

Okay, I think I have everything green for real this time. This stuff is not easy 😅

[chuckcarpenter]

had to remove the changes here b/c I changed this file to use more as a layout for more pages. The script is in the demo.astro file now, but worse case we capture those in another PR