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

chore: work around Ajv module bug in generated code #256

Merged
merged 1 commit into from
Sep 12, 2024

Conversation

EvanHahn
Copy link
Contributor

JavaScript module inconsistency strikes again.

In some cases, Ajv will inject require calls into generated code even if you ask it to generate ESM. This is a bug.

For example, here's something it adds if you use the string minLength or maxLength properties:

// ...
const func2 = require("ajv/dist/runtime/ucs2length").default;
// ...

That won't work in an ESM environment.

As a workaround, I inject the following into our generated code:

import { createRequire } from 'node:module';
const require = createRequire(import.meta.url);

This, along with moving ajv to production dependencies, should work around this issue until the bug is fixed.

This isn't a problem today, but will immediately become a problem if we start using one of those validators. It's a bug waiting to happen.

I think this is a useful change on its own, but it should hopefully make an upcoming change easier.

JavaScript module inconsistency strikes again.

In some cases, Ajv will inject `require` calls into generated code even
if you ask it to generate ESM. [This is a bug][0].

For example, here's something it adds if you use [the string `minLength`
or `maxLength` properties][1]:

    // ...
    const func2 = require("ajv/dist/runtime/ucs2length").default;
    // ...

That won't work in an ESM environment.

As a workaround, I inject the following into our generated code:

    import { createRequire } from 'node:module';
    const require = createRequire(import.meta.url);

This, along with moving `ajv` to production dependencies, should work
around this issue until the bug is fixed.

[0]: ajv-validator/ajv#2209
[1]: https://json-schema.org/understanding-json-schema/reference/string#length
Copy link
Contributor

@tomasciccola tomasciccola left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

deamn, nice catch!

@EvanHahn EvanHahn merged commit 149845f into main Sep 12, 2024
6 checks passed
@EvanHahn EvanHahn deleted the ajv-generated-code-workaround branch September 12, 2024 18:31
gmaclennan added a commit that referenced this pull request Sep 16, 2024
This is a different approach to the fix in #256 because that was not playing nicely with Rollup bundler.
gmaclennan added a commit that referenced this pull request Sep 16, 2024
This is a different approach to the fix in #256 because that was not playing nicely with Rollup bundler.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants