Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Typescipt and ESM yields "Ajv This expression is not constructable" #2132

Closed
cyberwombat opened this issue Oct 16, 2022 · 26 comments
Closed

Comments

@cyberwombat
Copy link

cyberwombat commented Oct 16, 2022

I have a TS project in ESM mode (type:module) and getting VSCode errors using certain modules such as Ajv..

Given this in my tsconfig.json:

{  
  "compilerOptions": {
    "module": "NodeNext",
    "moduleResolution": "nodenext",
    "esModuleInterop": true
    // I had a lot more stuff but the top two lines are the ones causing issue (Node16 is same)
  }
}

package.json

{
  "name": "test",
  "type": "module",
  "dependencies": {
    "ajv": "^8.11.0"
  },

  "devDependencies": {
    "@tsconfig/node16": "^1.0.3",
    "@types/node": "^18.11.0",
    "typescript": "^4.8.3"
  }
}

and finally code:

import Ajv from "ajv"
export const getClient = ():Ajv => { 
  return  new Ajv()
}

I get the error as attached
Screen Shot 2022-10-15 at 9 58 18 PM

Note that this code compiles correctly.

If I do:

import  Ajv from "ajv"
export const getClient = ():Ajv.default => { 
  return  new Ajv.default()
}

Then error goes away but it does not run.

I think this is something to do with CJS modules needing to add something for esm export but not sure exactly what.

Here's a sandbox

@kachkaev
Copy link

kachkaev commented Oct 25, 2022

Same problem here in a ESM package. Here is a temp workaround:

-import Ajv from "ajv";
+import _Ajv from "ajv";
+
+const Ajv = _Ajv as unknown as typeof _Ajv.default;

const ajv = new Ajv();

@cyberwombat
Copy link
Author

@kachkaev that now gives me errors about compile and other methods not existing on ajv client

@kachkaev
Copy link

kachkaev commented Oct 25, 2022

Hmm not sure why that would be. I’m running an experiment in blockprotocol/blockprotocol#709, things seem to work fine there. Well, if _Ajv as unknown as typeof _Ajv.default can be classified as ‘fine’ 😅

I’ve got allowSyntheticDefaultImports and esModuleInterop enabled in tsconfig, moduleResolution is equal to NodeNext. A few more CommonJS libs with a default export need the same hack.

@cyberwombat
Copy link
Author

My bad - I had the return type on the client creation set to typeof Ajv as per VS suggestion which was wrong.

@nicojs
Copy link

nicojs commented May 6, 2023

This is also working for me:

import ajvModule from 'ajv';
const Ajv = ajvModule.default;
const ajv = new Ajv();

(no need for an unknown type assertion)

@benasher44
Copy link
Contributor

It looks like this is still an issue with the latest version. See https://arethetypeswrong.github.io/?p=ajv%408.12.0

@robcresswell
Copy link

robcresswell commented Jun 23, 2023

@nicojs Did that work at runtime? Its not the same as what @kachkaev wrote, which is just adjusting the types. You're actually assigning to default, rather than just forcing the type inference, which the link provided by @benasher44 indicates this is incorrect.

@nicojs
Copy link

nicojs commented Jun 23, 2023

I'm always using --esModuleInterop, maybe that does the trick

@andrewbranch
Copy link

If I do:

import  Ajv from "ajv"
export const getClient = ():Ajv.default => { 
  return  new Ajv.default()
}

Then error goes away but it does not run.

This does run correctly in Node.js, as @nicojs pointed out. I don’t know of any runtime or bundler where it wouldn’t run. However, it’s probably not the API that AJV intended to give to Node.js ESM users.

In the runtime CJS code, AJV uses a clever interop pattern:

module.exports = exports = Ajv;
Object.defineProperty(exports, "__esModule", { value: true });
exports.default = Ajv;

that makes Ajv exposed both on module.exports and module.exports.default. So in reality, a Node ESM importer can do both of these:

// In Node, a default import always binds to the
// `module.exports` of a CJS module!
import Ajv from "ajv";
new Ajv();         // module.exports -> Ajv
new Ajv.default(); // module.exports.default -> Ajv

However, the types don’t reflect the existence of this interop pattern. The types just say

export default Ajv;

which implies that only this part exists at runtime:

Object.defineProperty(exports, "__esModule", { value: true });
exports.default = Ajv;

Which is why TypeScript is only going to let you access the symbol on module.exports.default:

import Ajv from "ajv";
new Ajv();
//  ^^^ The only thing you told me about the `module.exports` of "ajv"
//      is that it has a `default` property with an `Ajv` class.

new Ajv.default();
//  Yeah, now you’re constructing the class that you declared to exist.

To make the types more accurate, you should do something like this:

import AjvCore from "./core";
import { Format, FormatDefinition, AsyncFormatDefinition, KeywordDefinition, KeywordErrorDefinition, CodeKeywordDefinition, MacroKeywordDefinition, FuncKeywordDefinition, Vocabulary, Schema, SchemaObject, AnySchemaObject, AsyncSchema, AnySchema, ValidateFunction, AsyncValidateFunction, SchemaValidateFunction, ErrorObject, ErrorNoParams, } from "./types";
import { Plugin, Options, CodeOptions, InstanceOptions, Logger, ErrorsTextOptions } from "./core";
import { SchemaCxt, SchemaObjCxt } from "./compile";
import { KeywordCxt } from "./compile/validate";
import { DefinedError } from "./vocabularies/errors";
import { JSONType } from "./compile/rules";
import { JSONSchemaType } from "./types/json-schema";
import { _, str, stringify, nil, Name, Code, CodeGen, CodeGenOptions } from "./compile/codegen";
import { default as ValidationError } from "./runtime/validation_error";
import { default as MissingRefError } from "./compile/ref_error";
declare namespace Ajv {
  const _default: typeof Ajv;
  export { _default as default, Format, FormatDefinition, AsyncFormatDefinition, KeywordDefinition, KeywordErrorDefinition, CodeKeywordDefinition, MacroKeywordDefinition, FuncKeywordDefinition, Vocabulary, Schema, SchemaObject, AnySchemaObject, AsyncSchema, AnySchema, ValidateFunction, AsyncValidateFunction, SchemaValidateFunction, ErrorObject, ErrorNoParams, Plugin, Options, CodeOptions, InstanceOptions, Logger, ErrorsTextOptions, SchemaCxt, SchemaObjCxt, KeywordCxt, DefinedError, JSONType, JSONSchemaType, _, str, stringify, nil, Name, Code, CodeGen, CodeGenOptions, ValidationError, MissingRefError };
}
declare class Ajv extends AjvCore {
  _addVocabularies(): void;
  _addDefaultMetaSchema(): void;
  defaultMeta(): string | AnySchemaObject | undefined;
}
export = Ajv;

Notice that there’s now an export = Ajv instead of export default Ajv, but the namespace Ajv also has a default export/property with the same type.

@wjureczka
Copy link

Any updates with version v8?

@fredsensibill
Copy link

fredsensibill commented Dec 5, 2023

This still doesn't work. [email protected]

This is also not working for me:

import _Ajv from 'ajv';

const Ajv = _Ajv as unknown as typeof _Ajv.default;
const ajv = new Ajv();

I get an error TS2339: Property 'default' does not exist on type 'typeof Ajv'.

Only way I could make this work was with a @ts-ignore, unfortunately.

import _Ajv from 'ajv';

// @ts-ignore
const Ajv = _Ajv.default;
const ajv = new Ajv();

@cj-christoph-gysin
Copy link

cj-christoph-gysin commented Dec 7, 2023

This could easily be fixed by providing named exports for Ajv, and then import those:

import { Ajv } from 'ajv';

const ajv = new Ajv();

The existing default export can be left untouched to avoid breaking any backwards compatibility. I'd be happy to make a PR for that.

@andrewbranch
Copy link

FWIW, my comment above is PR-able and will fully fix the types. I didn’t PR it because I noticed that no meaningful changes have been made to this repo in a while, and I don’t use this library personally. But anyone else is welcome to copy/paste and ping me for review.

@benasher44
Copy link
Contributor

@andrewbranch I'm willing/ready to open a PR, but the changes you describe I think only work if we can hand edit the .d.ts. In this case, the .d.ts is generated by TypeScript. I have so far been unsuccessful in writing a namespace in valid TypeScript that similarly re-exports the imports inside the namespace, like you suggested. I agree though that this does fix it, and I can verify the fixes work, when hand editing the .d.ts locally.

@andrewbranch
Copy link

It looks like this will do the trick

class Ajv extends AjvCore {
+ static default = Ajv
  // ...
}

- module.exports = exports = Ajv
+ export = Ajv
Object.defineProperty(exports, "__esModule", {value: true})

- export default Ajv
- export {
-   Format,
-   FormatDefinition,
-   // ...
- } from "./types"

+ import * as types from "./types"
+ import * as core from "./core"
+ // ...

+ namespace Ajv {
+   export import Format = types.Format
+   export import FormatDefinition = types.FormatDefinition
+   // ...
+ }

@benasher44
Copy link
Contributor

Thanks for that! This gets very close. In resolve.ts, there's now an issue importing the Ajv class type.

Here's the import:

image

Here's an example usage:
image

@benasher44
Copy link
Contributor

Draft PR here: #2365

I'm not sure if there is a better way, but I ended up making named exports in two places to make their usages in export import easier.

@andrewbranch
Copy link

How about instead of the default static property, we do this:

export = Ajv
Object.defineProperty(Ajv, "__esModule", {value: true})
+ Object.defineProperty(Ajv, "default", {value: Ajv})

+ declare namespace Ajv {
+   export { Ajv as default };
+ }
namespace Ajv {
  export import Format = types.Format
}

The issue is that the static property only confers a value meaning, not the type meaning of the Ajv class. Namespace exports, unlike properties, confer all meanings of the original symbol. But export declarations (export { Ajv as default }) aren’t allowed in real, non-ambient namespaces (for some reason), so we have to put it in an ambient one, and then take care of the value side in a way that TS isn’t paying attention to (Object.defineProperty). Not beautiful, but it should work.

@benasher44
Copy link
Contributor

Adding just that seems to work, but then I go remove the static default on the class, and I get this:

image

@benasher44
Copy link
Contributor

benasher44 commented Jan 8, 2024

And this for each of the export import in the non-ambient namespace (the ts error, not the eslint one):

image

@andrewbranch
Copy link

That’s all eslint.

@benasher44
Copy link
Contributor

Whooops yes it lol 🤦

@benasher44
Copy link
Contributor

Published @benasher44/ajv, if folks want to give it a try

@piotr-cz
Copy link

piotr-cz commented May 6, 2024

This has been fixed in v8.13 via f4a4c8e:

import { Ajv } from 'ajv'

@mosoriorian
Copy link

mosoriorian commented Jul 19, 2024

Maybe it is possible to close this issue with a reference to this PR: #2389 ?

The issue should be resolved and we can now import Ajv as @piotr-cz mentioned

@jasoniangreen
Copy link
Collaborator

Thanks for the heads up @mosoriorian and @piotr-cz and anyone else involved. Closing as fixed in #2389

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment