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

fast-uri still not compatible for AJV #85

Open
2 tasks done
jasoniangreen opened this issue Jun 13, 2024 · 6 comments
Open
2 tasks done

fast-uri still not compatible for AJV #85

jasoniangreen opened this issue Jun 13, 2024 · 6 comments

Comments

@jasoniangreen
Copy link

jasoniangreen commented Jun 13, 2024

Prerequisites

  • I have written a descriptive issue title
  • I have searched existing issues to ensure the bug has not already been reported

Fastify version

2.4.0

Plugin version

No response

Node.js version

18.x

Operating system

macOS

Operating system version (i.e. 20.04, 11.3, 10)

14.5

Description

Hi there, I know you're all keen to get fast-uri compatible to be used as default in AJV. I setup the following test with the latest version and it still fails while uri-js passes.

const AJV = require('ajv');
const fastUri = require('fast-uri');
const ajv = new AJV({
  uriResolver: fastUri // comment this line to see it works with uri-js
});

const schema = {
  $ref: "#/definitions/Record%3Cstring%2CPerson%3E",
  definitions: {
    Person: {
      type: "object",
      properties: {
        firstName: {
          type: "string",
        },
      },
    },
    "Record<string,Person>": {
      type: "object",
      additionalProperties: {
        $ref: "#/definitions/Person",
      },
    },
  },
};

const data = {
  joe: {
    firstName: "Joe",
  },
};

const validate = ajv.compile(schema); // throws with fast-uri
console.log(validate(data));

Link to code that reproduces the bug

No response

Expected Behavior

Should log true with fast-uri just as it does with uri-js which can you see if you comment out the line that adds fast-uri.

@Uzlopak
Copy link
Contributor

Uzlopak commented Jun 13, 2024

Somehow strange if i fix the #ref to be $ref: "#/definitions/Record<string,Person>", then it passes in runkit

Added the unit tests of uri-js #86
And if we really try to be compatible with uri-js, then we fail by 11 tests...

@jasoniangreen
Copy link
Author

Somehow strange if i fix the #ref to be $ref: "#/definitions/Record<string,Person>", then it passes in runkit

Added the unit tests of uri-js #86 And if we really try to be compatible with uri-js, then we fail by 11 tests...

Yeah, I mean, I am confused as to why uri-js even succeeds with the encoded ref but ultimately it does and now we have to continue supporting it.

@gurgunday
Copy link
Member

Hmm, I will try to resolve this for good this weekend but we might have to create 2 modes ultimately

@Uzlopak suggested a full rewrite would probably be faster to support url-js

@jasoniangreen
Copy link
Author

I have released 8.17.1 of AJV with fast-uri after the recent fixes for browser support and another issue we found. No guarantees there aren't any others but I will stay on top of issues and hopefully if anything else comes up it can be fixed forward.

@mcollina
Copy link
Member

Awesome news @jasoniangreen!

@zekth
Copy link
Member

zekth commented Oct 21, 2024

@jasoniangreen based on the timeline of this issue and no comment so far i assume we can close it. Can you confirm?

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

No branches or pull requests

5 participants