ThomasAribart / json-schema-to-ts

Infer TS types from JSON schemas πŸ“
MIT License
1.47k stars 31 forks source link

`Unexpected token 'export'` when exporting `wrapValidatorAsTypeguard` with a ts-node build #97

Closed lcsmuller closed 1 year ago

lcsmuller commented 1 year ago

Hello, thanks for writing this fantastic library, it has been useful so far!

I'm getting the following error when building with ts-node:

$ cross-env NODE_ENV=development nodemon --exec ts-node -r dotenv-flow/config -r tsconfig-paths/register src/index.ts | pino-pretty -c
[nodemon] 2.0.19
[nodemon] to restart at any time, enter `rs`
[nodemon] watching path(s): *.*
[nodemon] watching extensions: ts,json
[nodemon] starting `ts-node -r dotenv-flow/config -r tsconfig-paths/register src/index.ts`
/home/lcsmuller/PF/quickstart-nodejs-rest/node_modules/json-schema-to-ts/lib/index.js:1
export { wrapCompilerAsTypeGuard, wrapValidatorAsTypeGuard, } from "json-schema-to-ts/lib/type-guards";
^^^^^^

SyntaxError: Unexpected token 'export'
    at Object.compileFunction (node:vm:352:18)
    at wrapSafe (node:internal/modules/cjs/loader:1033:15)
    at Module._compile (node:internal/modules/cjs/loader:1069:27)
    at Module._extensions..js (node:internal/modules/cjs/loader:1159:10)
    at Object.require.extensions.<computed> [as .js] (/home/lcsmuller/PF/quickstart-nodejs-rest/node_modules/ts-node/src/index.ts:1361:43)
    at Module.load (node:internal/modules/cjs/loader:981:32)
    at Function.Module._load (node:internal/modules/cjs/loader:822:12)
    at Module.require (node:internal/modules/cjs/loader:1005:19)
    at require (node:internal/modules/cjs/helpers:102:18)
    at Object.<anonymous> (/home/lcsmuller/PF/quickstart-nodejs-rest/src/common/schemas/utils/helpers.ts:3:1)
[nodemon] app crashed - waiting for file changes before starting...

This error only seems to trigger when I attempt to export a method that comes from a .js file (wrapValidatorAsTypeguard or wrapCompilerAsTypeguard)... It appears that ts-node cannot support export and import syntax by default, I think an easy fix for this would be targeting es5 instead (unless there is a rationale for not doing so). If you are busy, I can give it a try to write the PR.

ThomasAribart commented 1 year ago

@lcsmuller should be alright since v2.6.1 πŸ‘ Can you give it a shot ?

lcsmuller commented 1 year ago

Sure! I'm still getting the same error JS syntax error, but I've noticed you have now added transpiling it to esm/ and cjs/ directories, how do I make sure the .ts file I'm importing from will reference the .js files from esm/? I apologize for the newbie question!

yannickrocks commented 1 year ago

I am also getting the same error.

ThomasAribart commented 1 year ago

@lcsmuller Normally you shouldn't need to worry about that, it's done under the hood by node (I think ts-node will use commonjs imports though).

See https://webreflection.medium.com/cjs-vs-esm-5f8b90a4511a

TL:DR, cjs is the "old" way of importing JS, still needed for backward compatibility, esm is the "new" way. The package.json exposes two keys, main and module, so both code can be used.

Okay I'll investigate some more πŸ‘

azizghuloum commented 1 year ago

Hello,

There is a problem in the package itself I believe. Tried with NodeJS 19, 18, and 16. Tried with an empty project that depends only on json-schema-to-ts (no typescript, no nothing).

$ cat package.json
{
  "name": "testproj",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "author": "",
  "license": "ISC",
  "dependencies": {
    "json-schema-to-ts": "^2.6.1"
  }
}
$ cat index.js
const x = require("json-schema-to-ts"); 
$ node ./index.js
/private/tmp/testproj/node_modules/json-schema-to-ts/lib/cjs/index.js:1
export { wrapCompilerAsTypeGuard, wrapValidatorAsTypeGuard, asConst } from "./utils";
^^^^^^

SyntaxError: Unexpected token 'export'
    at internalCompileFunction (node:internal/vm:74:18)
    at wrapSafe (node:internal/modules/cjs/loader:1141:20)
    at Module._compile (node:internal/modules/cjs/loader:1182:27)
    at Module._extensions..js (node:internal/modules/cjs/loader:1272:10)
    at Module.load (node:internal/modules/cjs/loader:1081:32)
    at Module._load (node:internal/modules/cjs/loader:922:12)
    at Module.require (node:internal/modules/cjs/loader:1105:19)
    at require (node:internal/modules/cjs/helpers:103:18)
    at Object.<anonymous> (/private/tmp/testproj/index.js:2:11)
    at Module._compile (node:internal/modules/cjs/loader:1218:14)

Node.js v19.2.0

It doesn't seem that node understands that syntax (Is this really valid commonjs export syntax?).

Digging further, trying to load the package as ES module, not as CJS. I added "type": "module", to package.json and modified index.js to have be:

//const x = require("json-schema-to-ts");
import x from "json-schema-to-ts";

Still same error. Node still tries to load /lib/cjs/index.js which has the export syntax error.

Digging further in https://nodejs.org/api/packages.html#writing-dual-packages-while-avoiding-or-minimizing-hazards (didn't know there were hazards here!) ...

So, I modified node_modules/json-schema-to-ts/package.json and added the following:

  "exports": {
    "require": "./lib/cjs/index.js",
    "import": "./lib/esm/index.js"
  },

NOW using the import syntax in my project resolves to the lib/esm/index.js file and loads properly while using the require syntax in my project resolves to the lib/cjs/index.js file (and still loads with error).

Did I miss something very obvious here? Or is the package really broken (which is hard for me to believe).

azizghuloum commented 1 year ago

TLDR:

Try node --require json-schema-to-ts vs node --import json-schema-to-ts from the command line.

  1. They both resolve to the same file (which they shouldn't).
  2. Node cannot load that file.

Now hack your package.json to include:

  "exports": {
    "require": "./lib/cjs/index.js",
    "import": "./lib/esm/index.js"
  },

Now --import tries to import the ESM file and succeeds (Phew) while --require tries to import the CJS file and fails because of the invalid syntax.

azizghuloum commented 1 year ago

Looking at babel.config.js, and reading https://babeljs.io/docs/en/babel-preset-env#modules

You have:

  env: {
    cjs: {
      presets: [["@babel/preset-env", { modules: false }], ...defaultPresets],
    },
    esm: {
      presets: [["@babel/preset-env", { modules: "cjs" }], ...defaultPresets],
    },
  },

Aren't these simply backward?

azizghuloum commented 1 year ago

https://github.com/ThomasAribart/json-schema-to-ts/pull/110

ThomasAribart commented 1 year ago

@azizghuloum I actually think you're right, bad copy-paste on my side from another project :)

Can you remove the change in the package.json ? And I merge this right away

azizghuloum commented 1 year ago

Done and thanks for the great work BTW. πŸ‘

ThomasAribart commented 1 year ago

Deployed with version 2.6.2-beta.1, can you re-try and confirm that it works ?

lcsmuller commented 1 year ago

I can confirm it's working @ThomasAribart, thank you very much, and also @azizghuloum!

ThomasAribart commented 1 year ago

Nice πŸ‘ ! 2.6.2 is out so you can switch to an official version ! Cheers !