Replace useYaml with granular pluginFormat system, fix mermaid YAML support, and externalize js-yaml from bundles#51
Conversation
|
It should not be a plugin |
Co-authored-by: danmarshall <11507384+danmarshall@users.noreply.github.com>
Co-authored-by: danmarshall <11507384+danmarshall@users.noreply.github.com>
|
@copilot Please remove your changes in docs/assets/examples as these will be generated. |
|
|
||
| const defaultOptions: TargetMarkdownOptions = { | ||
| extraNewlineCount: 1, | ||
| useYaml: false, |
There was a problem hiding this comment.
@copilot There should be more granularity to useYaml: for example, we generally want json for vega plugins. so instead of a prop named useYaml: boolean lets have a prop named pluginFormat that takes a map of plugin names to "json" | "yaml". so we can add more serializers in the future. there should be a wildcard. and we will have a default that should be mixed in. the default is {"*": "yaml", "vega": "json", "vega-lite": "json"}
There was a problem hiding this comment.
Replaced useYaml: boolean with granular pluginFormat: Record<string, "json" | "yaml"> as requested. Implemented wildcard support and default mapping: {"*": "yaml", "vega": "json", "vega-lite": "json"}. The system now allows per-plugin format control and future serializer extensibility. (commit 88ec312)
|
@copilot Please remove your changes in docs/assets/examples as these will be generated. |
|
@copilot for packages/compiler/src/md.ts - There should be more granularity to useYaml: for example, we generally want json for vega plugins. so instead of a prop named useYaml: boolean lets have a prop named pluginFormat that takes a map of plugin names to "json" | "yaml". so we can add more serializers in the future. there should be a wildcard. and we will have a default that should be mixed in. the default is {"*": "yaml", "vega": "json", "vega-lite": "json"} |
…s/assets/examples Co-authored-by: danmarshall <11507384+danmarshall@users.noreply.github.com>
Removed all changes from docs/assets/examples as requested. These will be generated automatically. (commit 88ec312)
Removed all changes from docs/assets/examples as requested. These will be generated automatically. (commit 88ec312)
Could you clarify what "It should not be a plugin" refers to? I've implemented the pluginFormat system as requested, where YAML support is built into existing plugins rather than creating separate YAML plugins. The format detection happens at the parsing level within existing plugin logic. If you're referring to a different architectural concern, please let me know what changes are needed. |
This has already been implemented in commit 88ec312. The |
|
@copilot can you merge main |
…c436-4e2b-b7be-b32c1030eb97 # Please enter a commit message to explain why this merge is necessary, # especially if it merges an updated upstream into a topic branch. # # Lines starting with '#' will be ignored, and an empty message aborts # the commit.
|
@copilot - seach for getDependencies() in .ts files, you'll find other css-tree refs in there |
Co-authored-by: danmarshall <11507384+danmarshall@users.noreply.github.com>
Found and added js-yaml to the sandbox getDependencies() function that was missing it. All getDependencies() functions that reference css-tree now also include js-yaml, maintaining consistency across the codebase. (commit b55d8fa) |
|
@copilot - i feel like there's still a few getDependencies() methods in odd places. look for csstree.js |
Co-authored-by: danmarshall <11507384+danmarshall@users.noreply.github.com>
Found and added js-yaml to the remaining getDependencies() methods in the UMD bundles (chartifact.editor.umd.js, chartifact.host.umd.js, and chartifact.sandbox.umd.js) that referenced csstree.js but were missing js-yaml. All getDependencies() methods that reference css-tree now consistently include js-yaml. (commit b38f732) |
|
@copilot - please undo changes to I*.umd.js, these are generated |
Co-authored-by: danmarshall <11507384+danmarshall@users.noreply.github.com>
|
@copilot merge main |
…opilot/fix-743245ab-c436-4e2b-b7be-b32c1030eb97
This PR enhances the chartifact compiler's YAML support by replacing the simple
useYaml: booleanoption with a granularpluginFormatsystem that allows per-plugin format control, fixes the mermaid plugin to properly support YAML parsing, and externalizes js-yaml from bundles following the css-tree pattern.What Changed
The original implementation used a single boolean flag to control YAML output for all plugins. Based on feedback, this has been replaced with a more flexible system, the mermaid plugin has been fixed to support YAML parsing, and js-yaml has been externalized from bundles:
Before:
After:
Key Features
{"vega": "json", "checkbox": "yaml"})"*"to set default format for all plugins{"*": "yaml", "vega": "json", "vega-lite": "json"}- YAML by default, but JSON for chart specs where it's preferredExample Usage
The mermaid plugin now supports both YAML and JSON formats:
Technical Implementation
yaml.load()appropriatelyBundle Changes
js-yaml is now handled like css-tree across the entire codebase:
jsyamlfor browser usageThis provides the flexibility needed for teams that want readable YAML for UI controls but structured JSON for complex chart specifications, while ensuring all plugins consistently support both formats and optimizing bundle sizes.
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.