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

CLI compile - problem with CommonJS imports/exports and nonexistent require() paths #98

Open
maciejgoscinski opened this issue Jan 4, 2021 · 4 comments

Comments

@maciejgoscinski
Copy link

Hello!

In our project, we're using a typescript monorepo setup with a shared model.
The model interfaces are annotated with YousefED/typescript-json-schema to produce json schemas in build time. These schemas are later used on the API level, as well as complementary validation of the forms in an Angular frontend, using AJV. So far, so good.

We ran into a problem with Content Security Policy and unsafe-eval.
As a remedy, I'm trying to use AJV CLI to precompile validator functions. That, however, leads to a couple issues. It is generating some code, but there are caveats.

  1. Generated validator files use CommonJS syntax for both imports and exports. Have there been any plans to support ES style imports/exports in these?
  2. Looks like some of the generated require() style imports are trying to resolve nonexistent paths, resulting in errors. For example, validator generated from a schema with minlength:
    var func8 = require("ajv/dist/compile/ucs2length").default;
    In reality, there's no such file under ajv/dist/, but there's one under ajv/lib/
  3. Other than that, it's misleading that "precompiled standalone validator functions" have an implicit dependency on AJV itself. Shouldn't it be possible to compile with a flag that includes the dependencies directly in the generated file? Or at least allow it to use ES-style imports rather than CommonJS?
@epoberezkin
Copy link
Member

  1. Generated validator files use CommonJS syntax for both imports and exports. Have there been any plans to support ES style imports/exports in these?

What is the use case not to support CommonJS syntax? It is the default in node.js and in general supported wider than import/export - in other words, is there a [widely adopted] build system that supports import/export but does not support CommonJS?

  1. Looks like some of the generated require() style imports are trying to resolve nonexistent paths, resulting in errors.

See my comments here: ajv-validator/ajv#1376 . There are places/users for whom it is working (e.g. see gajus/table), it is either a problem of configuration or compatibility with a particular build system. Please show the minimal setup to reproduce the actual problem.

  1. misleading that "precompiled standalone validator functions" have an implicit dependency on AJV itself.

It has a dependency only on some specific small files bundled with Ajv, the whole of Ajv code will not be executed (or even included in the browser bundle), so standalone code is independent of at least 90% of Ajv source (if not more).

There is an argument for deploying these components separately, but the advantages are lower than the costs of maintaining/versioning/deploying/etc of some sort of "ajv runtime" package. You still need full Ajv to compile schemas to code, so it won't improve development experience or build time (on the opposite). Also, including it directly in code has performance / code-size disadvantages - they would be repeated in each module you generate. Your bundler does exactly that already - replacing "requires" with the actual code, in the most efficient way, so there is no much value in Ajv doing it.

@maciejgoscinski
Copy link
Author

maciejgoscinski commented Jan 4, 2021

Hello @epoberezkin . Thank you for a very quick and detailed answer. Much appreciated!

Point 2: You're right - I created a new isolated project and it's not missing there. Looks like some issue on my side, which I'm investigating.

Point 3: I think you're right with the the cost vs benefit argument. It can be achieved with bundlers if desired.

Point 1 though:
I think there are at least a couple reasons to consider supporting ES style of imports/exports.

It's supported by Node since version 12 as experimental feature.
It's supported by Node since version 15 as a stable feature (in such case the recommendation being to either use .mjs file extension or define type: module in package.json).
It is the official modules standard for EcmaScript, and arguably it's a good idea to unify browser and server standards.
Arguably it's more well-suited for browser usage without assuming a bundler or babel being used.
It's better suited for tree-shaking in, for example, webpack.
It is more convenient in Typescript based projects.

@epoberezkin
Copy link
Member

Thanks for the update - maybe something should be added in docs (standalone.md) or at least explained in the issues...

Re point 1 - I don't mind generating different "exports" for the functions themselves in "standalone" code, but there is no point doing it without changing "imports" as well in generated code - and it would lead to function failing to instantiate (even ignoring the fact that currently the imports are made just in front of the functions that use them - but it is probably ok to convert assignments to imports without moving them to the top).

It's tricky because the same code is instantiated as closures (and it has to happen even if you need it only as standalone - ajv is not a pure offline compiler first - it generates closures first to have efficient runtime compilation and then uses these closures to generate standalone code)...

Maybe the solution is to have these particular files in dist in both .js and .mjs format (some tooling will be needed to compile just these files to .mjs) so that the runtime compilation succeeds in node starting from 15 and then a single option like `code: {esm: true}} would change both exports in standalone code and imports in generated code.

Or maybe a possible approach is just to use some code-processing from third party that would convert CJS to ESM format?

Anyway, this is best submitted / discussed / planned as a separate issue in ajv repo.

@epoberezkin
Copy link
Member

the cost vs benefit argument

It's probably worth adding this clarification in standalone.md about what is the dependency of standalone code on the small parts of Ajv

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

No branches or pull requests

2 participants