fix type imports for nodejs esm

[rosmcmahon]

i have definitely not tested this any further than for my own purposes.

environment:

• node v18.15.0
• ts-node run with esm

error seen:

Could not find a declaration file for module 'warp-contracts/mjs'. '/Users/ros/repo-personal/test-nodejs-esm/node_modules/warp-contracts/lib/mjs/index.js' implicitly has an 'any' type.

[rosmcmahon]

i should probably mention, i'm importing like:
import { LoggerFactory, WarpFactory } from 'warp-contracts/mjs'

[ppedziwiatr]

Hey @rosmcmahon , why do you import like this?
For TS, it should be import { LoggerFactory, WarpFactory } from 'warp-contracts' ...

[rosmcmahon]

if i try from 'warp-contracts' it looks like it's defaulting to cjs for some reason:

error TS7016: Could not find a declaration file for module 'warp-contracts'. '/Users/ros/repo-personal/test-nodejs-esm/node_modules/warp-contracts/lib/cjs/index.js' implicitly has an 'any' type.

[ppedziwiatr]

I see, we will need to fix it then.

In the meantime, can you please also verify version 1.4.0. 1.4.1 introduced some changes in bundling (https://github.com/warp-contracts/warp/releases/tag/v1.4.1), which could potentially cause these issues...

[ppedziwiatr]

So I've prepared an example from a template (https://github.com/chriswells0/node-typescript-template) and it works fine:

Repo with example script is here: https://github.com/warp-contracts/ts-node-test/blob/master/src/index.ts
warp-contracts: 1.4.5

ts-node also seems to be working fine:

[rosmcmahon]

the above appears to be a cjs test?

"module": "commonjs",

https://github.com/warp-contracts/ts-node-test/blob/1d792162f6c93a49d8139f34bf6639a8abcf2c0f/tsconfig.json#L10

[rosmcmahon]

this is my test tsconfig.json:

{
  "$schema": "https://json.schemastore.org/tsconfig",
  "display": "Node v18 LTS ESM",
  "compilerOptions": {
    /* Modules */
    "lib": ["ES2022","DOM"],
    "module": "ES2022",
    "target": "ES2022",
    "moduleResolution":  "bundler",// "NodeNext",
    "outDir": "./build",

    /* Type Checking */
    "strict": true,
    "strictNullChecks": true, //ts_belt
    "noUncheckedIndexedAccess": true, //ts_belt
    // "typeRoots": ["./node_modules/@types", "./node_modules/warp-contracts/lib/types/index.d.ts"],

    /* Interop Constraints */
    "esModuleInterop": true,
    "forceConsistentCasingInFileNames": true, 
    // "resolveJsonModule": true,
    "allowSyntheticDefaultImports": true,

    /* Completeness */
    "skipLibCheck": true,
    "checkJs": true
  }
  ,
  "ts-node": {
    "esm": true,
    "experimentalSpecifierResolution": "node"
  }
}

[rosmcmahon]

incidentally, if i try specifying the typeRoots like commented out above, i get:

Could not find a declaration file for module 'warp-contracts'. '/Users/ros/repo-personal/test-nodejs-esm/node_modules/warp-contracts/lib/cjs/index.js' implicitly has an 'any' type.
  There are types at '/Users/ros/repo-personal/test-nodejs-esm/node_modules/warp-contracts/lib/types/index.d.ts', but this result could not be resolved when respecting package.json "exports". The 'warp-contracts' library may need to update its package.json or typings.ts(7016)

[ppedziwiatr]

Yes, this is also how tsc is configure in our SDK https://github.com/warp-contracts/warp/blob/main/tsconfig.json#L6
I believe that's the most common and 'default' setting, e.g. arweave-js is also using it - https://github.com/ArweaveTeam/arweave-js/blob/master/tsconfig.node.json#L10

Re. your config - what is
moduleResolution: "bundler" ?

The allowed values are:
["Classic", "Node", "Node16", "NodeNext"]

[ppedziwiatr]

ESM version warp-contracts/ts-node-test@effaa73

Works for me:

[rosmcmahon]

Yes, this is also how tsc is configure in our SDK https://github.com/warp-contracts/warp/blob/main/tsconfig.json#L6 I believe that's the most common and 'default' setting, e.g. arweave-js is also using it - https://github.com/ArweaveTeam/arweave-js/blob/master/tsconfig.node.json#L10

arweave-js is purely a commonjs library. your app can be esm or cjs and import it though. if your test app is set to "module": "commonjs" it's not an esm app afaik?

Re. your config - what is moduleResolution: "bundler" ?

The allowed values are: ["Classic", "Node", "Node16", "NodeNext"]

this is a new option since Typescript@5. once i updated to v5 i had import problems when using "NodeNext" for esm.
p.s. before you get excited, absolutely nothing is fixed in typescript v5 related to all of this 😅

[ppedziwiatr]

@asiaziola what do you think? I believe we've been here before and it was causing some fuckups for some of the web bundlers?

[asiaziola]

we can quickly test it i guess, can you release a beta version with the change so I can run tests on bundlers?

[asiaziola]

also - shouldn't we attach types to both of the versions?
so it will look sth like:

  "require": {
        "types": "./not-sure-whats-the-path-to-types",
        "default": "./lib/cjs/index.js"
      }

[ppedziwiatr]

warp-sdk is by default compiled to cjs format.
We also don't have type:module set in package.json - so in that sense it is also a cjs module.

But there is an mjs version - for the sake of using it from .mjs files. This version is not compiled from scratch - only a ESM wrapper is being created. This is also the reason why there is a separate package.json for the mjs version (with the 'type' set to 'module'). This is how official node.js docs suggest to do this (i.e. create a wrapper and not compile separately to cjs and esm - for reference https://nodejs.org/api/packages.html#dual-package-hazard).

As I've mentioned - this commit warp-contracts/ts-node-test@effaa73 works for me - is it not a viable solution for you?

Sorry for making such a big problem from such a small change, but our expierience is that such changes always break sth for someone else (usually for some web bundlers, like rollup, esbuild, next.js/vite-whatever-they-are-using-underneath, etc.)

[rosmcmahon]

no absolutely, these tiny changes are key.
if you check my first sentence above

i have definitely not tested this any further than for my own purposes.
i have not tested on web or cjs.

yeah, i'm interested in this hybrid packaging. i'm not a fan of this "rip the band aid off" approach of going full esm, and giving the finger to the rest of the js/ts ecosystem 😅

[rosmcmahon]

As I've mentioned - this commit warp-contracts/ts-node-test@effaa73 works for me - is it not a viable solution for you?

somehow skipped over this, let me check, thanks!

[rosmcmahon]

hmm, if change the moduleResolution in your test project we are back to the original error.
anyhow. this is not a big deal for me personally. i was just curious as to what was going on with the import and thought i saw a quick fix. 👍

feel free to close!

[ppedziwiatr]

We will play with it a bit more :-)

[ppedziwiatr]

I'll try with facebook/lexical#4160 (comment) - I guess we're not the only ones having issues with the new moduleResolution option...

[ppedziwiatr]

Though in this case it's probably more of a nolimits4web/swiper#6508

gxmari007/vite-plugin-eslint#60

[ppedziwiatr]

ok, so this one works with the new moduleResolution: bundler (no other changes are required):

"types": "./lib/types/index.d.ts",
  "main": "./lib/cjs/index.js",
  "browser": {
    "./lib/cjs/index.js": "./bundles/web.bundle.min.js"
  },
  "exports": {
    "./web": "./bundles/web.bundle.min.js",
    ".": {
      "require": {
        "types": "./lib/types/index.d.ts",
        "default": "./lib/cjs/index.js"
      },
      "import": {
        "types": "./lib/types/index.d.ts",
        "default": "./lib/mjs/index.js"
      }
    }
  },

[ppedziwiatr]

https://github.com/warp-contracts/warp/pull/410/files
1.4.6-beta.ppe.1

[rosmcmahon]

Agree to closing. I'm not sure I should be setting moduleResolution like that after all.

[ppedziwiatr]

Sorry, we've tried with different settings, but each time at least one of our e2e test (for the concrete webbundler) was failing.
We'll probably need to return to this with an increasing ts 5 adoption - but for now we're simply too overloaded with other stuff :/

[elliotsayes]

FYI this library is now broken with create vite React + TS + SWC