-
-
Notifications
You must be signed in to change notification settings - Fork 67
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
base: master
Are you sure you want to change the base?
Conversation
Thank you. It all fails because of some typescript change - the same happened in other places, the code needs to be updated to compile… |
Note the inclusion of [1] https://stackoverflow.com/questions/60151181/object-is-of-type-unknown-typescript-generics |
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! |
@epoberezkin could this PR be merged? (Or maybe @jessedc?) It looks like other teams are also having to work around this issue where |
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 |
Hey there @epoberezkin can you merge this PR, please? I had to use patch-package to get around this issue. My diff file
|
Any news on this? |
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)) |
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! |
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! |
Is there anything preventing the merge of this PR? |
What's the hold up? |
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
but apparently was forgotten to be added. Perhaps @jessedc can be of help here? |
It would be great to get this merged for those of us trying to pre-compile validators. |
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.
@epoberezkin Anything new about merging this PR ? Thanks. |
I will follow up on this with @epoberezkin. |
As of ajv@8.9.0, compiling to esm is now supported. This PR allows that functionality to be accessed.