ajv-validator / ajv

The fastest JSON schema Validator. Supports JSON Schema draft-04/06/07/2019-09/2020-12 and JSON Type Definition (RFC8927)
https://ajv.js.org
MIT License
13.47k stars 864 forks source link

Add named exports for main classes to improve types for NodeNext #2389

Closed benasher44 closed 2 months ago

benasher44 commented 3 months ago

What issue does this pull request resolve?

Users of TypeScript + ESM + NodeNext have difficulty using the default exported classes, which is the only way to consume them (#2132).

What changes did you make?

This additionally adds a named export, which side-steps this issues around default imports in that environment. This is a very lightweight alternative to #2365 to improve things for these users, even if it won't make this package pass attw.

Is there anything that requires more attention while reviewing?

I don't think so.

benasher44 commented 3 months ago

The idea is that once this ships, users having trouble with the default export can switch to the name export instead.

jasoniangreen commented 3 months ago

Thanks for coming up with a simpler solution, but just so I understand, why do you need to both add export to export class Ajv2019 extends AjvCore as well as adding the .Ajv2019 to exports? And will adding the export keyword conflict with module.exports = exports = Ajv2019?

benasher44 commented 3 months ago

Good questions!

For Ajv2019 (and Ajv2020), that was just for consistency. I don't use them, but without much context, it felt weird to not do it for the others. I'm happy undo those!

As for the module.exports bit, let's look at the final output (dist/ajv.js), for example. Here's the relevant snippet:

exports.Ajv = Ajv;
module.exports = exports = Ajv;
module.exports.Ajv = Ajv;

First, exports.Ajv = Ajv is set and appears to be emitted by tsc. The problem is that the next line module.exports = exports = Ajv immediately overwrites that. Without any changes, this would cause a named import of Ajv to crash at runtime, since the Ajv class itself does not come with a static prop (or otherwise) to support the named export. This module.exports = exports = Ajv line comes from ajv.ts directly, so tsc is forced to emit it (afaict), despite (and probably unbeknownst to tsc) that is overwrites the named export. To counteract that, I added module.exports.Ajv = Ajv on the following line, which effectively adds an Ajv prop to the class, re-enabling the named export.

jasoniangreen commented 3 months ago

Thanks for the explanation, I will discuss with EP and get back to you. It might take me a couple of days as I have another priority at the moment, but I will definitely get back to you soon.

jasoniangreen commented 2 months ago

I haven't forgotten you. Just trying to get a release together with updated deps (as it has been a while), and then I will follow up on this with EP.

benasher44 commented 2 months ago

No problem. Thanks!

jomaa-daniel commented 2 months ago

Waiting on this update as well. 🙏

jasoniangreen commented 2 months ago

@epoberezkin FYI

jasoniangreen commented 2 months ago

I spoke to EP, I will be preparing a release with this and general dependency updates over the coming days. He has asked me to do some very specific due diligence and tests to reduce the chance of any issues. We have to be super cautious given the huge user base that rely on AJV and the wide variety of ways it is used, imported and built into projects.

flowluap commented 2 months ago

FYI:

I used those two commits, to patch the package for us, pre release. I am still getting:

TS2709: Cannot use namespace  Ajv  as a type.

This is how I import it...

import Ajv from 'ajv';

Could you include a named export for Ajv, usable as type?

benasher44 commented 2 months ago

The expectation is you would change the import to:

import { Ajv } from 'ajv';

flowluap commented 2 months ago

@benasher44 thanks for the fast reply. I installed the latest PR regarding the topic:

 "ajv": "github:ajv-validator/ajv#pull/2365/head",

And that one seems to be working perfectly fine with


 "lib": ["es2023"],
 "module": "node16",
 "moduleResolution": "nodenext",

Looking forward to the new release, thanks for your work!

benasher44 commented 2 months ago

👍 can you try with this PR and the named import I suggested? I don't expect that #2365 will ship, at this point.

flowluap commented 2 months ago

With the PR installed, using the named import like that:

import { Ajv } from 'ajv';

/// TS2595:  Ajv  can only be imported by using a default import.

Working fine is the default:

import  Ajv  from 'ajv';
benasher44 commented 2 months ago

@flowluap I'm not sure what you're doing, but if I pull master, run npm i, then npm run build, then npm run pack, I get a usable package. From there, I installed the locally built package. With that installed, I can import { Ajv } from 'ajv' without issue.

ehaynes99 commented 1 month ago

For anyone who needs a workaround until this is released, this works and is typesafe. There are similar problems with ajv-errors and ajv-formats, and those haven't been published in forever.

import AjvLib from 'ajv'
import AjvErrorsLib from 'ajv-errors'
import AjvFormats from 'ajv-formats'

const { default: Ajv } = AjvLib
type Ajv = InstanceType<typeof Ajv>

const { default: ajvErrors } = AjvErrorsLib
const { default: addFormats } = AjvFormats

const ajv: Ajv = new Ajv({ allErrors: true, strict: false })
ajvErrors(ajv, { singleError: true })
addFormats(ajv)
emmenko commented 1 month ago

Thanks for fixing this! ❤️

Is there a plan when this might be released? 🙏

shumphrey commented 1 month ago

Now that this has been released, I'm having issues getting ajv-formats working with this change, does a similar change need to be made to ajv-formats?

benasher44 commented 1 month ago

What's the issue?

shumphrey commented 1 month ago

What's the issue?

Argument of type 'import("blank/node_modules/ajv/dist/ajv").Ajv' is not assignable to parameter of type 'import("blank/node_modules/ajv-formats/node_modules/ajv/dist/core").default

This is with various different versions of import addFormats from "ajv-formats";

Seems like the type has changed and is no longer compatible?

shumphrey commented 1 month ago

The old typescript code was:

import Ajv from "ajv";
import addFormats from "ajv-formats";

const ajv = new Ajv.default({ allowUnionTypes: true });
addFormats.default(ajv);

With the new release of ajv:

import { Ajv } from "ajv";
import addFormats from "ajv-formats";

const ajv = new Ajv({ allowUnionTypes: true });
addFormats.default(ajv); // this throws a typescript error

I've tried various combos of how we import ajv-formats, I think the typing of the two don't work together any more

benasher44 commented 1 month ago

Can you please post the typescript error?

benasher44 commented 1 month ago

This works fine for me with ajv-formats 3.01 and ajv 8.13.0. Can you please post a reproduction?

import { Ajv } from "ajv";
import addFormats from "ajv-formats";
addFormats.default(new Ajv());
shumphrey commented 1 month ago
npm ls ajv
│ ├─┬ ajv-formats@3.0.1
│ │ └── ajv@8.12.0
│ └── ajv@8.13.0

I believe ajv-formats was bringing in the old ajv, resetting the package-lock resolved it.