-
Notifications
You must be signed in to change notification settings - Fork 182
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
DON'T MERGE: User plugins proposal and prototype #497
Draft
blake-mealey
wants to merge
22
commits into
rojo-rbx:master
Choose a base branch
from
blake-mealey:user-plugins-proposal
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+1,306
−185
Draft
Changes from all commits
Commits
Show all changes
22 commits
Select commit
Hold shift + click to select a range
605314a
add first draft of proposal
blake-mealey 10e980c
reordered example
blake-mealey 1a66313
clarified source field
blake-mealey 5de7fc5
add todo
blake-mealey 469c294
added name property to plugin descriptions
blake-mealey 2976960
added basic plugin loading
blake-mealey daf1b57
added options support to plugin descriptions, error handling to plugi…
blake-mealey f4c9d23
removed duplicate code
blake-mealey 3df0071
almost got middleware working
blake-mealey f447557
hacked it to make it work lol
blake-mealey 7457120
rename data to contents, update plugin implementations in design doc
blake-mealey 8c3113c
added links to requests for plugin features
blake-mealey 551997b
add rojo plugin library to design doc
blake-mealey 90826c9
improve error handling in plugin loading
blake-mealey 17a1db6
read files using plugin library
blake-mealey cb18cfe
use context_with_cfs when loading plugins, restrict lua std libraries…
blake-mealey b1081cd
finish porting all middlewares
blake-mealey fa31815
added todo
blake-mealey 07cd441
add description of middleware usage
blake-mealey 9e28502
import plugins relative to project file
blake-mealey c324500
fixed middleware returns for example moonscript
blake-mealey 3cb03d3
removed debug print
blake-mealey File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,376 @@ | ||
# User Plugins Design Proposal | ||
|
||
## Background | ||
|
||
This is a design proposal for the long-standing [#55](https://github.com/rojo-rbx/rojo/issues/55) | ||
desire to add user plugins to Rojo for things like source file transformation and instance tree | ||
transformation. | ||
|
||
As discussed in [#55](https://github.com/rojo-rbx/rojo/issues/55) and as initially explored in | ||
[#257](https://github.com/rojo-rbx/rojo/pull/257), plugins as Lua scripts seem to be a good starting | ||
point. This concept reminded me of Rollup.js plugins. [Rollup](https://rollupjs.org/guide/en/) is a | ||
JS bundler which performs a similar job to Rojo in the JS world by taking a number of source files | ||
and producing a single output bundle. [Rollup | ||
plugins](https://rollupjs.org/guide/en/#plugins-overview) are written as JS files, and passed to the | ||
primary Rollup config file. Rollup then calls in to plugin "hooks" at special times during the | ||
bundling process. I have found Rollup plugins to be an excellent interface for both plugin | ||
developers and end-users and therefore I have based this proposal on their API. | ||
|
||
This proposal attempts to take advantage of the existing "middleware" system in Rojo, and give | ||
plugins the opportunity to: | ||
|
||
- Override the default choice for which internal middleware should be used for a given file path | ||
- Transform the contents of a file before consuming it | ||
|
||
## Project file changes | ||
|
||
Add a new top-level field to the project file format: | ||
|
||
- `plugins`: An array of [Plugin Descriptions](#plugin-description). | ||
- **Optional** | ||
- Default is `[]` | ||
|
||
### Plugin description | ||
|
||
Either a `String` value or an object with the fields: | ||
|
||
- `source`: A filepath to the Lua source file of the plugin or a URL to a GitHub repo, optionally | ||
followed by an `@` character and a Git tag. | ||
- **Required** | ||
- `options`: Any JSON dictionary. The options that will be passed to the plugin. | ||
- **Optional** | ||
- Default is `{}` | ||
|
||
In the case that the value is just a `String`, it is interpreted as an object with the `source` | ||
field set to its value, and `options` set to its default. | ||
|
||
### Example | ||
|
||
```json | ||
{ | ||
"name": "ProjectWithPlugins", | ||
"plugins": [ | ||
"local-plugin.lua", | ||
"github.com/owner/remote-plugin-from-tag@v1.0.0", | ||
"github.com/owner/remote-plugin-from-head", | ||
{ "source": "plugin-with-options.lua", "options": { "some": "option" } } | ||
], | ||
... | ||
} | ||
``` | ||
|
||
## Plugin scripts | ||
|
||
Plugin scripts should return a `CreatePluginFunction`: | ||
|
||
```luau | ||
-- Types provided in luau format | ||
|
||
type PluginInstance = { | ||
name: string, | ||
projectDescription?: (project: ProjectDescription) -> (), | ||
syncStart?: () -> (), | ||
syncEnd?: () -> (), | ||
resolve?: (id: string) -> string, | ||
middleware?: (id: string) -> string, | ||
load?: (id: string) -> string, | ||
} | ||
|
||
-- TODO: Define properly. This is basically just the JSON converted to Lua | ||
type ProjectDescription = { ... } | ||
|
||
type CreatePluginFunction = (options: {[string]: any}) -> PluginInstance | ||
``` | ||
|
||
In this way, plugins have the opportunity to customize their hooks based on the options provided by | ||
the user in the project file. | ||
|
||
## Plugin instance | ||
|
||
- `name` | ||
- **Required**: The name of the plugin that will be used in error messages, etc. | ||
- `projectDescription(project: ProjectDescription) -> ()` | ||
- **Optional**: Called with a Lua representation of the current project description whenever | ||
it has changed. | ||
- `syncStart() -> ()` | ||
- **Optional**: A sync has started. | ||
- `syncEnd() -> ()` | ||
- **Optional**: A sync has finished. | ||
- `resolve(id: string) -> string` | ||
- **Optional**: Takes a file path and returns a new file path that the file should be loaded | ||
from instead. The first plugin to return a non-nil value per id wins. | ||
- `middleware(id: string) -> string` | ||
- **Optional**: Takes a file path and returns a snapshot middleware enum to determine how Rojo | ||
should build the instance tree for the file. The first plugin to return a non-nil value per | ||
id wins. | ||
- `load(id: string) -> string` | ||
- **Optional**: Takes a file path and returns the file contents that should be interpreted by | ||
Rojo. The first plugin to return a non-nil value per id wins. | ||
|
||
## Plugin environment | ||
|
||
The plugin environment is created in the following way: | ||
|
||
1. Create a new Lua context. | ||
1. Add a global `rojo` table which is the entry point to the [Plugin library](#plugin-library) | ||
1. Initialize an empty `_G.plugins` table. | ||
1. For each plugin description in the project file: | ||
1. Convert the plugin options from the project file from JSON to a Lua table. | ||
1. If the `source` field is a GitHub URL, download the plugin directory from the repo with the | ||
specified version tag (if no tag, from the head of the default branch) into a local | ||
`.rojo-plugins` directory with the repo identifier as its name. It is recommended that users | ||
add `.rojo-plugins` to their `.gitignore` file. The root of the plugin will be called | ||
`main.lua`. | ||
1. Load and evaluate the file contents into the Lua context to get a handle to the | ||
`CreatePluginFunction` | ||
1. Call the `CreatePluginFunction` with the converted options to get a handle of the result. | ||
1. Push the result at the end of the `_G.plugins` table | ||
|
||
If at any point there is an error in the above steps, Rojo should quit with an appropriate error | ||
message. | ||
|
||
## Plugin library | ||
|
||
Accessible via the `rojo` global, the plugin library offers helper methods for plugins: | ||
|
||
- `toJson(value: any) -> string` | ||
- Converts a Lua value to a JSON string. | ||
- `readFileAsUtf8(id: string) -> string` | ||
- Reads the contents of a file from a file path using the VFS. Plugins should always read | ||
files via this method rather than directly from the file system. | ||
- `getExtension(id: string) -> boolean` | ||
- Returns the file extension of the file path. | ||
- `hasExtension(id: string, ext: string) -> boolean` | ||
- Checks if a file path has the provided extension. | ||
|
||
## Use case analyses | ||
|
||
To demonstrate the effectiveness of this API, pseudo-implementations for a variety of use-cases are | ||
shown using the API. | ||
|
||
### MoonScript transformation | ||
|
||
Requested by: | ||
|
||
- @Airwarfare in [#170](https://github.com/rojo-rbx/rojo/issues/170) | ||
- @dimitriye98 in [#55](https://github.com/rojo-rbx/rojo/issues/55#issuecomment-402616429) (comment) | ||
|
||
```lua | ||
local parse = require 'moonscript.parse' | ||
local compile = require 'moonscript.compile' | ||
|
||
return function(options) | ||
return { | ||
name = "moonscript", | ||
middleware = function(id) | ||
if rojo.hasExtension(id, 'moon') then | ||
if rojo.hasExtension(id, 'server.moon') then | ||
return 'lua_server' | ||
elseif rojo.hasExtension(id, 'client.moon') then | ||
return 'lua_client' | ||
else | ||
return 'lua_module' | ||
end | ||
end | ||
end, | ||
load = function(id) | ||
if rojo.hasExtension(id, 'moon') then | ||
local contents = rojo.readFileAsUtf8(id) | ||
|
||
local tree, err = parse.string(contents) | ||
assert(tree, err) | ||
|
||
local lua, err, pos = compile.tree(tree) | ||
if not lua then error(compile.format_error(err, pos, contents)) end | ||
|
||
return lua | ||
end | ||
end | ||
} | ||
end | ||
``` | ||
|
||
### Obfuscation/minifier transformation | ||
|
||
Requested by: | ||
|
||
- @cmumme in [#55](https://github.com/rojo-rbx/rojo/issues/55#issuecomment-794801625) (comment) | ||
- @blake-mealey in [#382](https://github.com/rojo-rbx/rojo/issues/382) | ||
|
||
```lua | ||
local minifier = require 'minifier.lua' | ||
|
||
return function(options) | ||
return { | ||
name = "minifier", | ||
load = function(id) | ||
if rojo.hasExtension(id, 'lua') then | ||
local contents = rojo.readFileAsUtf8(id) | ||
return minifier(contents) | ||
end | ||
end | ||
} | ||
end | ||
``` | ||
|
||
### Markdown to Roblox rich text | ||
|
||
```lua | ||
-- Convert markdown to Roblox rich text format implementation here | ||
|
||
return function(options) | ||
return { | ||
name = 'markdown-to-richtext', | ||
middleware = function(id) | ||
if rojo.hasExtension(id, 'md') then | ||
return 'json_model' | ||
end | ||
end, | ||
load = function(id) | ||
if rojo.hasExtension(id, 'md') then | ||
local contents = rojo.readFileAsUtf8(id) | ||
|
||
local frontmatter = parseFrontmatter(contents) | ||
local className = frontmatter.className or 'StringValue' | ||
local property = frontmatter.property or 'Value' | ||
|
||
local richText = markdownToRichText(contents) | ||
|
||
return rojo.toJson({ | ||
ClassName = className, | ||
Properties = { | ||
[property] = richText | ||
} | ||
}) | ||
end | ||
end | ||
} | ||
end | ||
``` | ||
|
||
### Load custom files as StringValue instances | ||
|
||
Requested by: | ||
|
||
- @rfrey-rbx in [#406](https://github.com/rojo-rbx/rojo/issues/406) | ||
- @Quenty in [#148](https://github.com/rojo-rbx/rojo/issues/148) | ||
|
||
```lua | ||
return function(options) | ||
options.extensions = options.extensions or {} | ||
|
||
return { | ||
name = 'load-as-stringvalue', | ||
middleware = function(id) | ||
local idExt = rojo.getExtension(id) | ||
for _, ext in next, options.extensions do | ||
if ext == idExt then | ||
return 'json_model' | ||
end | ||
end | ||
end, | ||
load = function(id) | ||
local idExt = rojo.getExtension(id) | ||
for _, ext in next, options.extensions do | ||
if ext == idExt then | ||
local contents = rojo.readFileAsUtf8(id) | ||
local jsonEncoded = contents:gsub('\r', '\\r'):gsub('\n', '\\n') | ||
|
||
return rojo.toJson({ | ||
ClassName = 'StringValue', | ||
Properties = { | ||
Value = jsonEncoded | ||
} | ||
}) | ||
end | ||
end | ||
end | ||
} | ||
end | ||
``` | ||
|
||
```json | ||
// default.project.json | ||
{ | ||
"plugins": [ | ||
{ "source": "load-as-stringvalue.lua", "options": { "extensions": {"md", "data.json"} }} | ||
] | ||
} | ||
``` | ||
|
||
### File system requires | ||
|
||
Requested by: | ||
|
||
- @blake-mealey in [#382](https://github.com/rojo-rbx/rojo/issues/382) | ||
|
||
```lua | ||
-- lua parsing/writing implementation here | ||
|
||
return function(options) | ||
local project = nil | ||
|
||
return { | ||
name = "require-files", | ||
projectDescription = function(newProject) | ||
project = newProject | ||
end, | ||
load = function(id) | ||
if rojo.hasExtension(id, 'lua') then | ||
local contents = rojo.readFileAsUtf8(id) | ||
|
||
-- This function will look for require 'file/path' statements in the source and replace | ||
-- them with Roblox require(instance.path) statements based on the project's configuration | ||
-- (where certain file paths are mounted) | ||
return replaceRequires(contents, project) | ||
end | ||
end | ||
} | ||
end | ||
``` | ||
|
||
<!-- ### Remote file requires | ||
|
||
This might be a terrible idea tbh (but still cool). | ||
|
||
```lua | ||
-- this pseudo-implementation doesn't really make sense atm as resolve would never be called with a | ||
-- URL like this | ||
|
||
-- download/caching implementation inline here | ||
|
||
return function(options) | ||
return { | ||
name = "remote-require", | ||
resolve = function(id) | ||
if id:match('^https?://.*%.lua$') then | ||
local cachedId = fromCache(id) | ||
return cachedId or nil | ||
end | ||
end, | ||
load = function(id) | ||
if id:match('^https?://.*%.lua$') then | ||
local cachedId = downloadAndCache(id) | ||
local file = io.open(cachedId, 'r') | ||
local source = file:read('a') | ||
file:close() | ||
return source | ||
end | ||
end | ||
} | ||
end | ||
``` --> | ||
|
||
## Implementation priority | ||
|
||
This proposal could be split up into milestones without all the features being present till the end | ||
to simplify development. Here is a proposal for the order to implement each milestone: | ||
|
||
1. Loading plugins from local paths | ||
1. Minimum Rojo plugin library (`readFileAsUtf8`) | ||
1. Calling hooks at the appropriate time | ||
1. Start with `middleware`, `load` | ||
1. Add `syncStart`, `syncEnd`, `resolve` | ||
1. Add `projectDescription` | ||
1. Full Rojo plugin library | ||
1. Loading plugins from remote repos |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Having a quick read over this doc, making plugins race against each other seems a bit odd, and could lead to some nondeterminism in different runs (plugin 1 was quick the first time round, but something happened and it was slow the second time, so its output is now lost).
Maybe it would be nicer to have a specific ordering of the plugins? the
plugins
field in the project file format even leads to this - a user can order the plugins in the array depending on how they want them to run. (e.g. we have plugin 1 which firstly transforms some code, then we let plugin 2 run afterwards which takes the output from plugin 1, and minifies it)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 quite sure where you got the idea that the plugins would race each other but your suggestion is actually how it's already intended to work. The
plugins
array allows the user to determine the plugin order and that order is used each time to deterministically determine the results.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 proposal said "the first plugin to return a non nil value wins" but maybe I misunderstood this - I thought it meant all plugins run at the same time and the first one to return a value will be used. I now realise I think it means that once a value is returned, it no longer calls for values from other plugins? My apologies!
Still, I think the second point mentioned may still make sense. If plugin 1 transforms require paths, and plugin 2 minifies the whole code, then we want to run both plugins (plugin 1 then 2), rather than just stopping after 1. I haven't looked into implementation details though so if this actually is what is happening already then my bad.
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.
Ahh I understand the confusion now. Yes, that sentence means "run them in order and stop once one has returned a value."
Yes, that is a good point! I will need to get my head back into how this proposal works (it's been a while) and come back with a response on it but that's definitely a use case that should be supported.