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

feat: add json schema for bslint.json #105

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

bartvandenende-wm
Copy link

@bartvandenende-wm bartvandenende-wm commented Apr 25, 2024

Summary

feat: add json schema for bslint.json and generate types from it

Details

  • creates a bslint.schema.json given the current supported properties and types supported by bslint
  • uses json2ts to auto generate the types from the schema to prevent duplicate maintenance.
  • add a npm prebuild step to ensure types are generated before TSC compilation

solves #104

How it was tested

  • reference the new schema in a local setup using
  "$schema": "./node_modules/@rokucommunity/bslint/bslint.schema.json",
  • running npm run build and npm run test in this project

@bartvandenende-wm bartvandenende-wm force-pushed the bartvandenende/bslint-json-schema branch from 36db77e to 7e6c4d0 Compare April 25, 2024 13:13
src/index.ts Outdated Show resolved Hide resolved
Copy link
Member

@TwitchBronBron TwitchBronBron left a comment

Choose a reason for hiding this comment

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

Hey, thanks for this.

I know it's slightly more work to keep the typescript interface and the schema in sync, but I'm not crazy about removing the interface from typescript and having it be generated, as that adds extra complexity when understanding how the project works (and we have so few contributors as it is).

Let's eliminate the src/generated concept and just maintain the schema and typescript interfaces separately. If you wanted, you could add a validation step that fails the build if the schema and interface are out of sync (which prevents us from releasing when they're out of sync).

@bartvandenende-wm bartvandenende-wm force-pushed the bartvandenende/bslint-json-schema branch from 1d52e5d to 1604b72 Compare May 13, 2024 19:19
@bartvandenende-wm
Copy link
Author

I know it's slightly more work to keep the typescript interface and the schema in sync, but I'm not crazy about removing the interface from typescript and having it be generated, as that adds extra complexity when understanding how the project works (and we have so few contributors as it is).

that's OK, note there is probably still opportunity to unify the TS types of BsLintConfig['rules'] and BsLintRules but that is outside the scope of this PR.

Let's eliminate the src/generated concept and just maintain the schema and typescript interfaces separately. If you wanted, you could add a validation step that fails the build if the schema and interface are out of sync (which prevents us from releasing when they're out of sync).

I added a test to validate both the BsLintConfig and json-schema against a fixture, to be able to load it from the test file from TS though I had to move the schema in the src folder.

@bartvandenende-wm bartvandenende-wm changed the title feat: add json schema for bslint.json and generate types from it feat: add json schema for bslint.json May 13, 2024
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