Skip to content

refactor keywordHandlers and errorHandlers #46

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

Merged
merged 2 commits into from
Aug 11, 2025

Conversation

arpitkuriyal
Copy link
Collaborator

@arpitkuriyal arpitkuriyal commented Aug 11, 2025

Created utilis.js file two reasons:

Circular Dependency: While some circular dependencies still exist, they have been successfully removed from index.js.

JSDoc Typing: The ErrorHandler type is needed, but it cannot be used in other files because it is not listed in index.d.ts. As a result, VS Code is unable to read the JSDoc types for index.js.

The getErrors function and the errorHandler array were moved to utilis.js. As the getErrors function is used within the error handlers, making it incorrect to import it from index.js.

The normalizeOutput file still needs to be refactored, but it's difficult because all the functions are interconnected. For example, evaluateSchema needs keywordHandlers, and keywordHandlers also uses evaluateSchema.

( we can change the file name of utilis.js as i was unable to think a better name for it )

Copy link
Collaborator

@jdesrosiers jdesrosiers left a comment

Choose a reason for hiding this comment

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

There were a few issues with this approach. I pushed some more refactoring. Here's what I did.

  • Pass a Localization object instead of the locale and create a new Localization object every time it's used.
  • Add handlers in index. This decouples the logic from the handlers being used. This allows us to, for example, configure for different JSON Schema dialects.
  • Expose functions to add handlers. This allows for custom dialects to add support for their custom keywords.
  • Changed the order of the arguments of the main function. It feels more intuitive this way.
  • Some minor naming changes and directory restructuring.

@jdesrosiers jdesrosiers merged commit da06c80 into hyperjump-io:main Aug 11, 2025
1 check passed
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