-
Notifications
You must be signed in to change notification settings - Fork 167
feat(bloblang): Improve Bloblang playground with Go/WASM migration #570
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: main
Are you sure you want to change the base?
Conversation
- Replaced JavaScript integration with Go and WASM instead - Improved WASM API exposure and updated JS integration - Added full syntax support to formatter (operators, keywords, pipes) Signed-off-by: Ramtin Mesgari <26694963+iamramtin@users.noreply.github.com>
Signed-off-by: Ramtin Mesgari <26694963+iamramtin@users.noreply.github.com>
…lidation API - Moved documentation HTML generation from JS to Go - Pre-generate docHTML for functions/methods - Added validation function - Added JSON utilities (format/minify/validate) with WASM and HTTP endpoints - Updated JS to prioritise WASM with native fallbacks - Centralized WASM function registration Signed-off-by: Ramtin Mesgari <26694963+iamramtin@users.noreply.github.com>
- Tests core functions: execute, validate, syntax, format, autocomplete - Tests JSON utilities: formatJSON, minifyJSON, validateJSON Signed-off-by: Ramtin Mesgari <26694963+iamramtin@users.noreply.github.com>
- Replace hardcoded values with CSS variables - Remove duplicate CSS rules - Fix mobile responsive layout - Add Bento-themed scrollbars to all editors Signed-off-by: Ramtin Mesgari <26694963+iamramtin@users.noreply.github.com>
Also simplified WASM initialization and auto-register server/WASM handlers. Signed-off-by: Ramtin Mesgari <26694963+iamramtin@users.noreply.github.com>
- Remove redundant JSON utilities from Go/WASM (use native JS instead) - Extract BloblangAPI abstraction layer for WASM/server routing - Remove unused code and dead methods - Improve error handling and WASM initialization flow - Improve UX Signed-off-by: Ramtin Mesgari <26694963+iamramtin@users.noreply.github.com>
0b9da37 to
0d0b520
Compare
- Fix type mismatch in getCompletions() for functionSpecWithHTML/methodSpecWithHTML - Improve autocompletion suggestions for Bloblang keywords - Add tests to verify completion types and structure Signed-off-by: Ramtin Mesgari <26694963+iamramtin@users.noreply.github.com>
0d0b520 to
fe4aa15
Compare
a6dd66b to
4e96b2c
Compare
Signed-off-by: Ramtin Mesgari <26694963+iamramtin@users.noreply.github.com>
4e96b2c to
b98ab91
Compare
|
Hey @gregfurman @jem-davies just a friendly ping to see if we can get this merged in? |
|
@iamramtin hey! Thanks for the ping and sorry for delay, will give this a priority review 👍 |
gregfurman
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.
I'll do another pass but have left some comments on structure and the pattern(s) used. Lmk thoughts!
internal/cli/blobl/core.go
Outdated
| return AutocompletionResponse{ | ||
| Completions: []CompletionItem{}, | ||
| Success: false, | ||
| Error: fmt.Sprintf("Failed to get syntax data: %v", err), |
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.
| Error: fmt.Sprintf("Failed to get syntax data: %v", err), | |
| Error: fmt.Sprintf("Failed to get syntax data: %s", err), |
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.
Why should we use %s here for error types?
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 sorry. Meant to say err.Error(), since that would extract the string form of the error:
| Error: fmt.Sprintf("Failed to get syntax data: %v", err), | |
| Error: fmt.Sprintf("Failed to get syntax data: %s", err.Error()), |
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.
Are these not effectively the same thing when the error is not nil?
Makefile
Outdated
|
|
||
| playground-test: | ||
| @go test ./internal/cli/blobl -timeout 3m |
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.
Wont this just run as a part of the normal test suite? Why do we need to add an extra make command?
internal/cli/blobl/wasm.go
Outdated
| // | ||
| // Arguments: | ||
| // - args[0]: JSON string containing AutocompletionRequest with line, column, prefix, beforeCursor | ||
| // | ||
| // Returns a JS object with: | ||
| // - "completions": array of completion items with caption, value, meta, type, score, docHTML | ||
| // - "success": true if autocompletion succeeded, false otherwise | ||
| // - "error": error message if autocompletion failed, else nil |
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 don't think this level of verbosity is necessary
| // | |
| // Arguments: | |
| // - args[0]: JSON string containing AutocompletionRequest with line, column, prefix, beforeCursor | |
| // | |
| // Returns a JS object with: | |
| // - "completions": array of completion items with caption, value, meta, type, score, docHTML | |
| // - "success": true if autocompletion succeeded, false otherwise | |
| // - "error": error message if autocompletion failed, else nil |
internal/cli/blobl/wasm.go
Outdated
| // Excludes functions that depend on system resources (env vars, filesystem). | ||
| var getWasmEnvironment = sync.OnceValue(func() *bloblang.Environment { | ||
| return bloblang.GlobalEnvironment().WithoutFunctions("env", "file") | ||
| }) |
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.
Already mentioned, but why not we replace this with an init and run the InitializeWASM logic as well as all JS function registration when the package gets initialised i.e
func init() {
env := bloblang.GlobalEnvironment().WithoutFunctions("env", "file")
InitializeWASM()
// .. pass 'env' around
}Then we can import
internal/cli/blobl/types.go
Outdated
| type FormatMappingResponse struct { | ||
| Success bool `json:"success"` | ||
| Formatted string `json:"formatted"` | ||
| Error string `json:"error,omitempty"` |
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 think we can use the globally available Error function from the JS environment i.e
var errorResp FormatMappingResponse
err := js.Global().Get("Error").New(errorResp.Error)
panic(err) // or maybe ignore if errorThen we can treat the error case as a JS-level error and handle with a try-catch instead of this error payload approach.
Also, that way we don't need to pass around an error attribute and can focus on payload responses.
wdyt?
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.
See 24c6fa92f2a9, lmk wyt!
internal/cli/blobl/types.go
Outdated
|
|
||
| // FunctionSpecWrapper wraps query.FunctionSpec to implement Spec interface | ||
| type FunctionSpecWrapper struct { | ||
| query.FunctionSpec |
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 don't think the entire struct needs to be embedded here since you're just using some attributes. Consider
| query.FunctionSpec | |
| spec query.FunctionSpec |
and then
func (f FunctionSpecWrapper) GetDescription() string { return f.spec.Description }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.
Good call, I think this explicitness is much better 👍
internal/cli/blobl/types.go
Outdated
| } | ||
|
|
||
| // Spec represents a unified interface for function and method specs | ||
| type Spec interface { |
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.
Why not use a dedicated Spec struct here instead with the attributes we need? Feels like 2 more structs + an interface is a bit of overkill.
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.
How would you feel about me adding a BaseSpec to internal/bloblang/query/docs.go and having FunctionSpec and MethodSpec embed those shared attributes instead of duplicating all this logic in this file?
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.
See commit 407ffa62b and LMK what you think, this is non-breaking and fully compatible and should let me not have to redeclare these shared attributes
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.
And follow up 1da4e310a to see how it affects the current code
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.
Wdyt of placing all of these WASM playground related functions in their own package i.e cli/blobl/wasm or /playground?
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.
See 2ee4747ac
…nd MethodSpec Signed-off-by: Ramtin Mesgari <26694963+iamramtin@users.noreply.github.com>
Signed-off-by: Ramtin Mesgari <26694963+iamramtin@users.noreply.github.com>
Signed-off-by: Ramtin Mesgari <26694963+iamramtin@users.noreply.github.com>
Signed-off-by: Ramtin Mesgari <26694963+iamramtin@users.noreply.github.com>
Signed-off-by: Ramtin Mesgari <26694963+iamramtin@users.noreply.github.com>
|
@gregfurman addressed comments, LMK WYT 🏁 |
Summary
Improves the Bloblang playground by migrating core logic from JavaScript to Go/WASM.
The playground previously duplicated some of Bloblang logic in JavaScript, creating maintenance overhead and potential inconsistencies. By moving to Go/WASM, we:
Changes
Go/WASM Migration:
JavaScript Simplification:
Other Improvements:
Testing
1. Run Unit Tests
2. Test Server Mode (Go backend via HTTP)
Visit playground and verify:
3. Test WASM Mode (Client-side execution)
Visit the playground in the docs and verify:
Breaking Changes
None