[experiment] Use DOM parser for SVG exports #3020
Closed
Conversation
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
steveruizok
temporarily deployed
to
deploy-staging
GitHub Actions
Inactive
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Ignored Deployment
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
steveruizok
temporarily deployed
to
deploy-staging
GitHub Actions
Inactive
|
Regarding the warnings above:
|
steveruizok
temporarily deployed
to
deploy-staging
GitHub Actions
Inactive
|
Weirdly this requires writing namespaces by hand? |
steveruizok
temporarily deployed
to
deploy-staging
GitHub Actions
Inactive
|
It looks like we'll have to add the namespace ourselves if we want this to work. |
|
Another thing we could do is make the |
steveruizok
temporarily deployed
to
deploy-staging
GitHub Actions
Inactive
1 task
steveruizok
changed the title
github-merge-queue bot
pushed a commit
that referenced
this pull request
Mar 25, 2024
## Migration path 1. If any of your shapes implement `toSvg` for exports, you'll need to replace your implementation with a new version that returns JSX (it's a react component) instead of manually constructing SVG DOM nodes 2. `editor.getSvg` is deprecated. It still works, but will be going away in a future release. If you still need SVGs as DOM elements rather than strings, use `new DOMParser().parseFromString(svgString, 'image/svg+xml').firstElementChild` ## The change in detail At the moment, our SVG exports very carefully try to recreate the visuals of our shapes by manually constructing SVG DOM nodes. On its own this is really painful, but it also results in a lot of duplicated logic between the `component` and `getSvg` methods of shape utils. In #3020, we looked at using string concatenation & DOMParser to make this a bit less painful. This works, but requires specifying namespaces everywhere, is still pretty painful (no syntax highlighting or formatting), and still results in all that duplicated logic. I briefly experimented with creating my own version of the javascript language that let you embed XML like syntax directly. I was going to call it EXTREME JAVASCRIPT or XJS for short, but then I noticed that we already wrote the whole of tldraw in this thing called react and a (imo much worse named) version of the javascript xml thing already existed. Given the entire library already depends on react, what would it look like if we just used react directly for these exports? Turns out things get a lot simpler! Take a look at lmk what you think This diff was intended as a proof of concept, but is actually pretty close to being landable. The main thing is that here, I've deliberately leant into this being a big breaking change to see just how much code we could delete (turns out: lots). We could if we wanted to make this without making it a breaking change at all, but it would add back a lot of complexity on our side and run a fair bit slower --------- Co-authored-by: huppy-bot[bot] <128400622+huppy-bot[bot]@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR gradually moves our shapes'
toSvgmethods to use DOM parsing instead of manually creating elements.Pros:
Cons:
Change Type
minor— New featureTest Plan
Release Notes