ajv-validator / ajv-cli

Command-line interface for Ajv JSON Validator
https://ajv.js.org
MIT License
262 stars 66 forks source link

Add `--code-esm` support #200

Open willfarrell opened 2 years ago

willfarrell commented 2 years ago

As of ajv@8.9.0, compiling to esm is now supported. This PR allows that functionality to be accessed.

epoberezkin commented 2 years ago

Thank you. It all fails because of some typescript change - the same happened in other places, the code needs to be updated to compile…

willfarrell commented 2 years ago

Note the inclusion of "useUnknownInCatchVariables": false. I tried the other patterns suggested from [1], but they still errored when using err.code (Property 'code' does not exist on type 'Error'.)

[1] https://stackoverflow.com/questions/60151181/object-is-of-type-unknown-typescript-generics

bitjson commented 2 years ago

PR reviewed and tested 👍 (I didn't realize this PR was already open when I sent https://github.com/ajv-validator/ajv-cli/pull/203).

@epoberezkin is there anything else that needs to happen before this can be merged? Thanks!

bitjson commented 2 years ago

@epoberezkin could this PR be merged? (Or maybe @jessedc?) It looks like other teams are also having to work around this issue where ajv-cli is missing the esm option from ajv. Or maybe is there some other way you'd prefer that the option be supported by ajv-cli?

trevorr commented 2 years ago

I would like to second the request for merging this. It's not easily usable directly from the fork repo.

Update: In case it helps anyone else, I forked the fork to swap prepublish with prepare, which allows use directly from the source repo: github:trevorr/ajv-cli#code-esm-prepare.

Creskendoll commented 2 years ago

Hey there @epoberezkin can you merge this PR, please? I had to use patch-package to get around this issue.

My diff file ajv-cli+5.0.0.patch

diff --git a/node_modules/ajv-cli/dist/commands/options.js b/node_modules/ajv-cli/dist/commands/options.js
index 23493ad..5d8c8c7 100644
--- a/node_modules/ajv-cli/dist/commands/options.js
+++ b/node_modules/ajv-cli/dist/commands/options.js
@@ -26,6 +26,7 @@ const ajvOptions = {
     multipleOfPrecision: boolOrNat,
     messages: { type: "boolean" },
     [`${CODE}es5`]: { type: "boolean" },
+    [`${CODE}esm`]: { type: "boolean" },
     [`${CODE}lines`]: { type: "boolean" },
     [`${CODE}optimize`]: boolOrNat,
     [`${CODE}formats`]: { type: "string" },
jonyw4 commented 2 years ago

Any news on this?

Creskendoll commented 2 years ago

If you're having this issue, I recommend you to either use patch-package as I described above, or do a similar thing without using the cli:

import standaloneCode from 'ajv/dist/standalone/index.js'
import glob from 'glob'
import Ajv from 'ajv'

// Read the .json schemas
const definitions = glob('validations/**/*.json', { sync: true })
const files = await Promise.all(definitions.map(async (f) => JSON.parse((await readFile(f)).toString())))

// Use Ajv with esm set to true 
const ajv = new Ajv({
  schemas: files,
  code: {
    source: true,
    esm: true,
  },
})

// Compile and save the code
writeFileSync('out.js', standaloneCode(ajv))
sondreb commented 1 year ago

Sorry for just 👍🏼 bumping this PR, but what is the holdback for getting the merge done? ESM is getting more and more popular and having to copy and run some code is not optimal solution. Thanks!

yelper commented 1 year ago

Thread bump; this simple change would enable pipelines to generate ESM schema instead of baking generation code into the runtime. It would unlock massive perf gains for us!

MarcDOLF commented 1 year ago

Is there anything preventing the merge of this PR?

dRoskar commented 1 year ago

What's the hold up?

MatthiasKunnen commented 1 year ago

I believe the PR has escaped @epoberezkin's attention since this CLI option is already documented at https://ajv.js.org/standalone.html#two-step-process

... or pass the --code-esm (CLI) flag if you want ESM exported code.

but apparently was forgotten to be added.

Perhaps @jessedc can be of help here?

jimkang commented 1 year ago

It would be great to get this merged for those of us trying to pre-compile validators.

damienflament commented 4 months ago

@epoberezkin Anything new about merging this PR ? Thanks.

jasoniangreen commented 3 months ago

I will follow up on this with @epoberezkin.