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

standalone code doesn't work when ajv-formats and user code use different ajvs (solution here) #1390

Closed
medikoo opened this issue Jan 13, 2021 · 13 comments

Comments

@medikoo
Copy link

medikoo commented Jan 13, 2021

I followed the advise at #1386 to generate standalone instance of validate. Still after having that done, occasionally we observe the following error when we try to run a generated validate

TypeError: formats4 is not a function
    at validate11 (/Users/medikoo/npm-packages/serverless/lib/classes/ConfigSchemaHandler/[generated-ajv-validate]9f269cb9d1ac9b6f0a2e48d232cc957067c41a6f.js:73:24798)

I looked into (prettified version) of generated code, and indeed something doesn't look right there.

At https://gist.github.com/medikoo/1d5f509c8d705b835c207fe1d3b37ef8#file-ajv-compiled-js-L9437-L9439 it's obviously the undefined that's assigned to formats4 variable

Then at L84237 (not shown in gist as file is too large), there's a if (!formats4(data331)) { which obviously crashes with above error, and I don't see in generated script any logic that will recover formats4 to be a function

Our standalone generator code can be inspected here:

https://github.com/serverless/serverless/blob/acd3b5b331c19fd70f620cc0b204992f6c21819a/lib/classes/ConfigSchemaHandler/resolveAjvValidate.js#L23-L34

What version of Ajv are you using? Does the issue happen if you use the latest version? v7.0.3

Ajv options object

{
      allErrors: true,
      coerceTypes: 'array',
      verbose: true,
      strict: false,
      code: { source: true },
}

JSON Schema

It's a large Schema as generated by Serverless Framework (we can provide a gist if needed)

@medikoo
Copy link
Author

medikoo commented Jan 13, 2021

As I investigated further this issue is characteristic to setup where dependencies are linked. In my case it's:

node_modules/ajv -> /usr/local/lib/node_modules/ajv (v7.0.3)
node_modules/ajv-formats -> /usr/local/lib/node_modules/ajv-formats (v1.5.1)

And /usr/local/lib/node_modules/ajv-formats has it's own installation of ajv (v7.0.3) in its node_modules.

If I install the dependencies normally via npm install, then standalone is generated without errors

@epoberezkin
Copy link
Member

epoberezkin commented Jan 13, 2021

Does it mean we can close it? To make it work in your case it may be enough to do npm link ajv (and/or ajv-formats in the right folder), if the issue is the code not finding it.

@medikoo
Copy link
Author

medikoo commented Jan 13, 2021

Does it mean we can close it? To make it work in your case it may be enough to do npm link ajv (and/or ajv-formats in the right folder), if the issue is the code not finding it.

There's no issue with installation on my side. Each package sees its each dependency at expected version.

It's a definitive issue on AJV (or bundler it uses) side. Maybe it's simply lack of support for symlinked dependencies (of which resultion works perfectly in scope of Node.js process).

This blocks us from shipping, as due to dynamic nature of our schema we generate it on user side, and now if user relies on some installer that favors symlinking (as e.g. pnpm) it may simply not work

@epoberezkin
Copy link
Member

Ok, it would help to have a reproducible small example to investigate/fix

@medikoo
Copy link
Author

medikoo commented Jan 13, 2021

Sure, I've published reproduction case here: https://github.com/medikoo/ajv-compilation-issue

@epoberezkin epoberezkin changed the title Standalone crashes with "TypeError: formats4 is not a function" standalone code doesn't work when ajv-formats and user code use different ajvs (solution here) Jan 16, 2021
@epoberezkin
Copy link
Member

epoberezkin commented Jan 16, 2021

@medikoo the reason for the problem is that ajv that is used by ajv-formats and ajv that is used by your code are different ajv's (you can check in node_modules), and because of that instanceof _Code that code generation uses to differentiate between strings and code snippets fails - ajv-formats uses one class, and ajv you use from your code uses another class (with the same name).

There are two solutions possible here:

  1. use package.json with all dependencies you need - not sure what is the reason to avoid it - it would make ajv-formats use the same code of ajv, as used in your code.
  2. manually assign ajv.opts.code.formats = _`require("ajv-formats/dist/formats").fullFormats` (or .fastFormats) in your code, where _ can be imported from ajv (const {default: Ajv, _} = require("ajv") in JS, import Ajv, {_} from "ajv" in TS)

I will think whether it is something that should be solved in ajv (e.g. replacing instanceof with some other check).

@medikoo
Copy link
Author

medikoo commented Jan 16, 2021

I will think whether it is something that should be solved in ajv (e.g. replacing instanceof with some other check).

From my experience instanceof quite often proves to be not reliable, and I personally do not resort to it

In Node.js there are a lot of ways to introduce a setup where two different installations of same package are installed (even if not linking it can easily be introduced when practicing fixed versioning in package.json).

In browsers there can be different realms (frames) and e.g. array instanceof Array will not work for array coming from different realm (it's a sole reason why Array.isArray was introduced)

IMO best is to rely on some kind of duck typing (all types detection in native layer works like that), with symbols it can be done really cleanly. Check Symbol.for which ensures same symbol instances across realms (actually this API was created to address this use case)

@epoberezkin
Copy link
Member

A possible fix is to pass _ and potentially other code-generation functions via the instance to ensure that the dependencies use the same class. So ajv-formats and ajv-keywords would use ajv._ etc.

@medikoo
Copy link
Author

medikoo commented Jan 18, 2021

use package.json with all dependencies you need - not sure what is the reason to avoid it - it would make ajv-formats use the same code of ajv, as used in your code.

This is the case, where standalone will be built on user side. Any installation that works for Node.js should be considered valid

manually assign ajv.opts.code.formats = _require("ajv-formats/dist/formats").fullFormats (or .fastFormats) in your code, where _ can be imported from ajv (const {default: Ajv, } = require("ajv") in JS, import Ajv, {} from "ajv" in TS

We can explore, still can you point exactly the code we should use? Our standalone generation code can be seen here: https://github.com/serverless/serverless/blob/acd3b5b331c19fd70f620cc0b204992f6c21819a/lib/classes/ConfigSchemaHandler/resolveAjvValidate.js#L23-L34 With what it should be replaced, to be bulletproof?

@epoberezkin
Copy link
Member

epoberezkin commented Jan 19, 2021

as I wrote above you need to do:

require('ajv-formats').default(ajv);
const {_} = require("ajv")
ajv.opts.code.formats = _`require("ajv-formats/dist/formats").fullFormats`

@epoberezkin
Copy link
Member

As long as your compiled code can require "ajv-formats/dist/formats" it would work - if not, you can change require path to simply point to this file location on your filesystem.

@epoberezkin
Copy link
Member

Now that ajv-formats has ajv as peer dependency (starting from v1.6 and in v2) it should not happen.

@medikoo
Copy link
Author

medikoo commented Jan 7, 2022

Just as a note I still find it happening with the latest version of ajv and ajv-format. If there's interest I can provide reproduction steps.

However, the patch proposed in the comment above works.

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