ajv

ajv copied to clipboard

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.

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

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?

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.

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.

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.

No problem. Thanks!

Waiting on this update as well. 🙏

@epoberezkin FYI

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.

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?

The expectation is you would change the import to:

import { Ajv } from 'ajv';

@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!

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

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';

@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.

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)

Thanks for fixing this! ❤️

Is there a plan when this might be released? 🙏

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?

What's the issue?

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?

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

Can you please post the typescript error?

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());

npm ls ajv
│ ├─┬ [email protected]
│ │ └── [email protected]
│ └── [email protected]

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