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

Add --code-esm support #200

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

willfarrell
Copy link

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

@epoberezkin
Copy link
Member

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
Copy link
Author

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
Copy link

bitjson commented Apr 14, 2022

PR reviewed and tested 👍 (I didn't realize this PR was already open when I sent #203).

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

@bitjson
Copy link

bitjson commented May 17, 2022

@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
Copy link

trevorr commented Jun 16, 2022

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
Copy link

Creskendoll commented Jul 1, 2022

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
Copy link

jonyw4 commented Jul 20, 2022

Any news on this?

@Creskendoll
Copy link

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
Copy link

sondreb commented Nov 23, 2022

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
Copy link

yelper commented Jan 5, 2023

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
Copy link

Is there anything preventing the merge of this PR?

@dRoskar
Copy link

dRoskar commented May 20, 2023

What's the hold up?

@MatthiasKunnen
Copy link

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
Copy link

jimkang commented Aug 7, 2023

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

mcdurdin added a commit to keymanapp/keyman that referenced this pull request Oct 5, 2023
ajv seems to have some bugs around precompiling and formats, particularly
when using ESM:

* ajv-validator/ajv#1837
* ajv-validator/ajv-cli#200

Workaround for non-esm works for cloudevents/sdk-javascript#471

So if we want to work with this, we should look at how that is done, but
it is taking too many cycles to pursue for now.
@damienflament
Copy link

@epoberezkin Anything new about merging this PR ? Thanks.

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

Successfully merging this pull request may close these issues.

None yet