-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Add JSON schema to extensions package.json #14918
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
base: master
Are you sure you want to change the base?
Add JSON schema to extensions package.json #14918
Conversation
martin-fleck-at
left a comment
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.
Hi @abelpz, thank you very much for that change! I really like the idea of extending the package.json with our own properties a lot! It's probably even a good idea to do it for the ApplicationProps as well some time in the future.
However, I do run into some troubles with the schema you provided. It does not really work for me in VS Code so I'm not sure this is a valid schema file to extend the existing package.json. I made some comments inline, could you please double check this?
| "description": "This helps people discover your package as it's listed in 'npm search'. Must include 'theia-extension'.", | ||
| "type": "array", | ||
| "items": { | ||
| "type": "string" | ||
| }, |
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.
Not sure if we need to re-specify these properties for keywords since it already exists in the base schema.
| "$schema": "http://json-schema.org/draft-07/schema#", | ||
| "title": "Theia package.json Schema", | ||
| "type": "object", | ||
| "$ref": "https://json.schemastore.org/package.json", |
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.
I believe the schema must follow something like this to properly extend the package.json:
"$schema": "http://json-schema.org/draft-07/schema#",
"title": "Theia package.json Schema",
"type": "object",
"allOf": [
{
"$ref": "https://json.schemastore.org/package"
},
{
"required": [
"keywords",
"theiaExtensions"
],
"properties": {
...
}
}
]
}
| "contains": { | ||
| "const": "theia-extension" | ||
| }, | ||
| "minItems": 1 |
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.
The error I get when not fulfilling this is Array does not contain required item. which might be confusing if you do not know the schema. Not sure if we really want to enforce this but I'm open to it.
| "description": "Path to the secondary window module" | ||
| } | ||
| }, | ||
| "additionalProperties": false |
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.
I'm not sure whether we want to be so restrictive but I'm open to it.
|
I would also need you to sign the ECA and your commit before we can do any merge of this. Thank you! |
sdirix
left a comment
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.
Drive-by comment: The descriptions are not fully correct.
| "properties": { | ||
| "frontend": { | ||
| "type": "string", | ||
| "description": "Path to the frontend module (used in browser and electron if frontendElectron is not provided)" |
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.
| "description": "Path to the frontend module (used in browser and electron if frontendElectron is not provided)" | |
| "description": "Path to the frontend module (used in browser and electron if frontendElectron and frontendOnly are both not provided)" |
| }, | ||
| "frontendPreload": { | ||
| "type": "string", | ||
| "description": "Path to the frontend preload module" |
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.
| "description": "Path to the frontend preload module" | |
| "description": "Path to the frontend preload module (used if frontendOnlyPreload is not provided)" |
|
Great @martin-fleck-at @sdirix. Thank you for replying to this, I appreciate all your comments. I didn't know much about how to describe this schema so I needed your help to define it better. I'll take a look at your reviews. |
|
Hi @abelpz, this PR has been open for some time without recent activity, just wondering if you are still planning to continue with it? TIA :) |
What it does
This PR introduces a custom JSON schema for Theia extension's
package.jsonfiles. The schema extends the standardpackage.jsonschema while adding proper validation and IntelliSense support for Theia-specific properties such astheiaExtensionsThis enhancement improves the developer experience by providing:
How to test
package.json:{ "$schema": "https://raw.githubusercontent.com/eclipse-theia/theia/master/schemas/theia-extension.schema.json", ... }package.jsonfile in VS Code or TheiatheiaExtensionsconfigurationFollow-ups
Breaking changes
Attribution
Review checklist
Reminder for reviewers