ThomasAribart / json-schema-to-ts

Infer TS types from JSON schemas 📝
MIT License
1.44k stars 31 forks source link

`wrapCompilerAsTypeGuard` and `wrapValidatorAsTypeGuard` are not usable in Deno build #74

Closed curtislarson closed 2 years ago

curtislarson commented 2 years ago

Hello! First off thanks for writing this library! It's awesome and just keeps getting better!

One thing I noticed is in the Deno build the wrapper functions cannot be used because its packaged as only type definitions and the implementations are stripped by rollup

https://github.com/ThomasAribart/json-schema-to-ts/blob/d8faafb4603aab1e0496858bd727f591940cc137/builds/deno/index.d.ts#L307

https://github.com/ThomasAribart/json-schema-to-ts/blob/d8faafb4603aab1e0496858bd727f591940cc137/builds/deno/index.d.ts#L312

Trying to import it and use it throws

error: SyntaxError: The requested module 'https://deno.land/x/json_schema_to_ts@v2.5.1/index.d.ts' does not provide an export named 'wrapCompilerAsTypeGuard'

I fixed it by just adding the implementation real quick and changing it to a .ts file since Deno doesnt always play nice with .d.ts files. That may not work the best with your rollup setup though and im sure you know a more compatible fix!

ThomasAribart commented 2 years ago

Hi @curtislarson ! Good catch, thanks for the heads up.

It seems that I now export more than types but forgot to modify the rollup configuration for deno export: https://github.com/ThomasAribart/json-schema-to-ts/blob/master/rollup.config.js

I'm gonna look into that !

ThomasAribart commented 2 years ago

Hi @curtislarson ! I found https://www.npmjs.com/package/@rollup/plugin-typescript which seems to bundle the typescript in cjs or esm.

I could use it along with dts to export an index.js in parallel to the index.d.ts. BUT I'm not sure if this is the best approach. Wouldn't it be better to simply export a single index.ts file ? Sorry, I'm not familiar with Deno.

curtislarson commented 2 years ago

@ThomasAribart The .js and .d.ts is a valid approach that some libraries like esbuild take with the <reference types notation.

I think most Deno people prefer importing from ts files since they may not be aware of the typing inclusion in the js file or would have to do the awkward @deno-types annotation. Definitely make sure it's ESM though and people can import it directly from github / unpkg / denoland / esm etc

Although honestly it feels like there will be a shift back to js extensions soon with typescript never allowing .ts imports and the shift away from typechecking in Deno. That's a bit too off topic though :)

curtislarson commented 2 years ago

https://gist.github.com/curtislarson/0253736b428c641411183cc41dec3202 This is pretty much what the file I use looks like, where you just import type/export type instead of import/export and include the function definitions.

Although you can go even further and use the optimized pinned urls from skypack...

ThomasAribart commented 2 years ago

@curtislarson Yes, that's what I had in mind. It sounds easy but it is in fact quite hard to automate :/ I couldn't find a rollup plugin that flattens ts like that. It seems simpler to export it as d.ts on one side and .js or .mjs on another.

curtislarson commented 2 years ago

I haven't experimented with them a lot but there are packages like unplugin and unbuild that seem to give you a lot of flexibility and additional options when it comes to generating output with rollup, esbuild, etc.

ThomasAribart commented 2 years ago

Sorry, those plugin don't allow to flatten ts filte easily either.

I've release a beta of v2.5.2 with the same d.ts as usual, but with js and mjs rollups containing both wrappers in cjs and esm. Can you try those and tell me if it works for you ? If it does, I'll officially release the 2.5.2.

https://deno.land/x/json_schema_to_ts@v2.5.2-beta.0

curtislarson commented 2 years ago

@ThomasAribart I'm able to import them but have to manually specify the correct typings via @deno-types in order to get it working.

Screen Shot 2022-05-26 at 12 44 41 PM Screen Shot 2022-05-26 at 12 44 51 PM

Which you can resolve by just reference the types in the .mjs/.js file. You can also import all the typings via the .mjs file when it's setup like this.

Screen Shot 2022-05-26 at 12 45 56 PM
ThomasAribart commented 2 years ago

The banner option seems to have done the job 👍

Can you try the second beta ? https://deno.land/x/json_schema_to_ts@v2.5.2-beta.1

curtislarson commented 2 years ago

@ThomasAribart Yep this code worked for me

import {
  wrapCompilerAsTypeGuard,
  $Compiler,
  JSONSchema,
} from "https://deno.land/x/json_schema_to_ts@v2.5.2-beta.1/index.mjs";

import Ajv from "https://esm.curtis.land/ajv@8.11.0/dist/2020";

const instance = new Ajv();

const $compile: $Compiler = (schema: JSONSchema) => instance.compile(schema);

const wrapped = wrapCompilerAsTypeGuard($compile);

const validate = wrapped({
  type: "object",
  required: ["name"],
  properties: { name: { type: "string" } },
} as const);

const res = validate({ name: "Curtis" });
console.log(res); // true

I don't think the .js version will ever be compatible with Deno though unless exports is shimmed but the .mjs worked great.

ThomasAribart commented 2 years ago

Awesome 😎

Does the type guard work as well ?

if (validate(data)) {
  ... // data should be correctly typed here
}

If so, I'll release and close this issue tomorrow 👍

Cheers !

curtislarson commented 2 years ago

Yep it works perfect. Thanks for the fixes!

ThomasAribart commented 2 years ago

Fixed in https://deno.land/x/json_schema_to_ts@v2.5.2