- Notifications
You must be signed in to change notification settings - Fork5
Update config files to use plain JS instead of JSON#149
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Pull Request Overview
This PR updates configuration files from JSON to plain JavaScript in order to simplify code, enable synchronous loading, and support browser contexts.
- Removed asynchronous JSON file reading and replaced it with ES module imports of JS config files.
- Introduced new config files: style/accessibility.js and style/base.js.
- Updated index.js to use these JS configs in a synchronous init function.
Reviewed Changes
Copilot reviewed 4 out of 6 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| test/accessibility-rules.test.js | Updated to import the JS accessibility config. |
| style/base.js | New file exporting the base config object. |
| style/accessibility.js | New file exporting the accessibility config object. |
| index.js | Updated to import JS configs and use a synchronous init. |
Files not reviewed (2)
- style/accessibility.json: Language not supported
- style/base.json: Language not supported
Comments suppressed due to low confidence (1)
index.js:9
- The init function is now synchronous; please verify that all consumers expecting a Promise are updated accordingly, or document this breaking change.
export function init(consumerConfig) {Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Thanks for making the change! I was hoping there's some cool way to connect JSON files/JSON Schema/directly importing JSON in ESM but that doesn't seem to be a thing yet.
c5d7531 intomainUh oh!
There was an error while loading.Please reload this page.
This package was recently updated to read the JSON files asynchronously using Node
fs. This makes the plugin unusable in browser contexts, breaking linting that would run in a browser likehttps://github.com/iansan5653/github-markdown-a11y-extension.While we can't (yet) import JSON directly in browsers using ESModules, we can just import JS. There's no reason these config fileshave to be JSON, and by changing them to JS we can simplify the code and change the extension back to a synchronous method. It should also be faster.