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

Implement Extensions System #262

Merged
merged 8 commits into from
Mar 7, 2022
Merged

Conversation

LeNitrous
Copy link
Member

Base implementation for extensions. Closes #251.

@LeNitrous LeNitrous added the priority:high Bugs or Features that requires immediate attention. label Mar 5, 2022
@LeNitrous LeNitrous self-assigned this Mar 5, 2022
@LeNitrous LeNitrous marked this pull request as ready for review March 5, 2022 10:50
@LeNitrous LeNitrous requested a review from sr229 March 5, 2022 12:04
- removed `Newtonsoft.Json` in favor of `System.Text.Json`
- refactored `VendorExtension` to allow for testing
- refactored file-backed vendor extensions
- renamed `BuiltIn` to `Host`
- implement basic vendor APIs
- add tests
Copy link
Member

@sr229 sr229 left a comment

Choose a reason for hiding this comment

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

You might wanna document how VFS is used to provide filesystem access to the host in Vignette.

@LeNitrous
Copy link
Member Author

VFS is a part of Stride. They have documentations for it on their end.

@sr229
Copy link
Member

sr229 commented Mar 7, 2022

VFS is a part of Stride. They have documentations for it on their end.

We're not talking about the VFS, we're talking about how we use it. Is this still in-line with our spec in #251?

@LeNitrous
Copy link
Member Author

For the most part yes specially with handling of the metadata.

However, I've taken a few liberties in the scripting API and loading the entry point script. A minimal working example is basically an ES6 Javascript file with two exports named activate and deactivate respectively. Additionally, the developer can access the core by importing the vignette module.

import { vignette } from "vignette";

export function activate() { }

export function deactivate() { }

This PR is only the bare minimum and does not fully implement the specification. Other parts of the API will be added in different PRs at a later time after consideration and discussion.

Copy link
Member

@sr229 sr229 left a comment

Choose a reason for hiding this comment

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

We'll do a proper RFC to further standardize this so we don't end up in dev hell. Otherwise, LGTM for a initial build.

@sr229 sr229 merged commit 796ff07 into vignetteapp:encore Mar 7, 2022
@LeNitrous LeNitrous deleted the feature/extensions branch March 7, 2022 04:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-extensions priority:high Bugs or Features that requires immediate attention.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Extension System
2 participants