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.

Sign up for GitHub

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

Jump to bottom

layers support in watch mode #1503

Merged
merged 4 commits into from
Jul 29, 2023
Merged

layers support in watch mode #1503

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7bea22e
draft
alixander Jul 29, 2023
65f97fc
pr comments
alixander Jul 29, 2023
2ca39f8
fmt
alixander Jul 29, 2023
11a4363
changelog
alixander Jul 29, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions ci/release/changelogs/next.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ Layout capability also takes a subtle but important step forward: you can now cu
- Configure timeout value with D2_TIMEOUT env var [#1392](https://github.com/terrastruct/d2/pull/1392)
- Scale renders and disable fit to screen with `--scale` flag [#1413](https://github.com/terrastruct/d2/pull/1413)
- `null` keyword can be used to un-declare. See [docs](https://d2lang.com/tour/overrides#null) [#1446](https://github.com/terrastruct/d2/pull/1446)
- Develop multi-board diagrams in watch mode (links to layers/scenarios/steps work in `--watch`) [#1503](https://github.com/terrastruct/d2/pull/1503)

#### Improvements 🧹

Expand Down
11 changes: 8 additions & 3 deletions d2cli/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,7 @@ func Run(ctx context.Context, ms *xmain.State) (err error) {
ctx, cancel := timelib.WithTimeout(ctx, time.Minute*2)
defer cancel()

_, written, err := compile(ctx, ms, plugins, layoutFlag, renderOpts, fontFamily, *animateIntervalFlag, inputPath, outputPath, *bundleFlag, *forceAppendixFlag, pw.Page)
_, written, err := compile(ctx, ms, plugins, layoutFlag, renderOpts, fontFamily, *animateIntervalFlag, inputPath, outputPath, "", *bundleFlag, *forceAppendixFlag, pw.Page)
if err != nil {
if written {
return fmt.Errorf("failed to fully compile (partial render written): %w", err)
Expand Down Expand Up @@ -366,7 +366,7 @@ func LayoutResolver(ctx context.Context, ms *xmain.State, plugins []d2plugin.Plu
}
}

func compile(ctx context.Context, ms *xmain.State, plugins []d2plugin.Plugin, layout *string, renderOpts d2svg.RenderOpts, fontFamily *d2fonts.FontFamily, animateInterval int64, inputPath, outputPath string, bundle, forceAppendix bool, page playwright.Page) (_ []byte, written bool, _ error) {
func compile(ctx context.Context, ms *xmain.State, plugins []d2plugin.Plugin, layout *string, renderOpts d2svg.RenderOpts, fontFamily *d2fonts.FontFamily, animateInterval int64, inputPath, outputPath, boardPath string, bundle, forceAppendix bool, page playwright.Page) (_ []byte, written bool, _ error) {
start := time.Now()
input, err := ms.ReadPath(inputPath)
if err != nil {
Expand Down Expand Up @@ -500,7 +500,12 @@ func compile(ctx context.Context, ms *xmain.State, plugins []d2plugin.Plugin, la
}
}

boards, err := render(ctx, ms, compileDur, plugin, renderOpts, inputPath, outputPath, bundle, forceAppendix, page, ruler, diagram)
board := diagram.GetBoard(boardPath)
if board == nil {
return nil, false, fmt.Errorf("Diagram with path %s not found", boardPath)
}

boards, err := render(ctx, ms, compileDur, plugin, renderOpts, inputPath, outputPath, bundle, forceAppendix, page, ruler, board)
if err != nil {
return nil, false, err
}
Expand Down
8 changes: 3 additions & 5 deletions d2cli/static/watch.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,7 @@ function init(reconnectDelay) {
const d2SVG = window.document.querySelector("#d2-svg-container");

const devMode = document.body.dataset.d2DevMode === "true";
const ws = new WebSocket(
`ws://${window.location.host}${window.location.pathname}watch`
);
const ws = new WebSocket(`ws://${window.location.host}/watch`);
let isInit = true;
let ratio;
ws.onopen = () => {
Expand All @@ -28,7 +26,7 @@ function init(reconnectDelay) {
// we can't just set `d2SVG.innerHTML = msg.svg` need to parse this as xml not html
const parsedXML = new DOMParser().parseFromString(msg.svg, "text/xml");
d2SVG.replaceChildren(parsedXML.documentElement);
changeFavicon("./static/favicon.ico");
changeFavicon("/static/favicon.ico");
const svgEl = d2SVG.querySelector("#d2-svg");
// just use inner SVG in watch mode
svgEl.parentElement.replaceWith(svgEl);
Expand Down Expand Up @@ -60,7 +58,7 @@ function init(reconnectDelay) {
if (msg.err) {
d2ErrDiv.innerText = msg.err;
d2ErrDiv.style.display = "block";
changeFavicon("./static/favicon-err.ico");
changeFavicon("/static/favicon-err.ico");
d2ErrDiv.scrollIntoView();
}
};
Expand Down
20 changes: 16 additions & 4 deletions d2cli/watch.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"os"
"path/filepath"
"runtime"
"strings"
"sync"
"time"

Expand Down Expand Up @@ -49,6 +50,7 @@ type watcherOpts struct {
port string
inputPath string
outputPath string
boardPath string
pwd string
bundle bool
forceAppendix bool
Expand Down Expand Up @@ -362,7 +364,7 @@ func (w *watcher) compileLoop(ctx context.Context) error {
w.pw = newPW
}

svg, _, err := compile(ctx, w.ms, w.plugins, w.layout, w.renderOpts, w.fontFamily, w.animateInterval, w.inputPath, w.outputPath, w.bundle, w.forceAppendix, w.pw.Page)
svg, _, err := compile(ctx, w.ms, w.plugins, w.layout, w.renderOpts, w.fontFamily, w.animateInterval, w.inputPath, w.outputPath, w.boardPath, w.bundle, w.forceAppendix, w.pw.Page)
errs := ""
if err != nil {
if len(svg) > 0 {
Expand Down Expand Up @@ -430,15 +432,25 @@ func (w *watcher) handleRoot(hw http.ResponseWriter, r *http.Request) {
<meta charset="UTF-8">
<meta name="viewport" content="width=device-width, initial-scale=1.0">
<title>%s</title>
<script src="./static/watch.js"></script>
<link rel="stylesheet" href="./static/watch.css">
<link id="favicon" rel="icon" href="./static/favicon.ico">
<script src="/static/watch.js"></script>
<link rel="stylesheet" href="/static/watch.css">
<link id="favicon" rel="icon" href="/static/favicon.ico">
</head>
<body data-d2-dev-mode=%t>
<div id="d2-err" style="display: none"></div>
<div id="d2-svg-container"></div>
</body>
</html>`, filepath.Base(w.outputPath), w.devMode)

// if path is "/x.svg", we just want "x"
boardPath := strings.TrimPrefix(r.URL.Path, "/")
if idx := strings.LastIndexByte(boardPath, '.'); idx != -1 {
boardPath = boardPath[:idx]
}
if boardPath != w.boardPath {
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the purpose of this check? Shouldn't we always render the requested board path?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

prevent duplication of compile requests on the initial load

Copy link
Contributor

Choose a reason for hiding this comment

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

Not clear to me how this accomplishes that. Can you elaborate? Also, fairly certain this is racey, I don't think you can modify w.boardPath like that without putting it under a mutex as the compilation occurs in the background.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Can you elaborate?

I just figure on initial load it's already compiling (behavior before this PR). So I don't want to add another recompile on top of that

fairly certain this is racey

it calls requestCompile(), which dodges races with channels.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

will address in another PR if needed

Copy link
Contributor

Choose a reason for hiding this comment

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

That doesn't dodge the race. You can't modify a variable that's read in another goroutine without synchronization. The channel send merely guarantees that the compileLoop goroutine will run a compile at some point in the future, not that it isn't already running one.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

i did go install -race ., then reran the manual test in the PR description, and it didn't say there was a race.

Copy link
Contributor

Choose a reason for hiding this comment

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

The race detector never produces false positives, but it does produce false negatives. It can't prove your program doesn't have races, only that it does.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

will look into this again later #1508

w.boardPath = boardPath
w.requestCompile()
}
}

func (w *watcher) handleWatch(hw http.ResponseWriter, r *http.Request) error {
Expand Down
67 changes: 67 additions & 0 deletions d2target/d2target.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"hash/fnv"
"math"
"net/url"
"os"
"strings"

"oss.terrastruct.com/util-go/go2"
Expand Down Expand Up @@ -72,6 +73,72 @@ type Diagram struct {
Steps []*Diagram `json:"steps,omitempty"`
}

// boardPath comes in the form of "x/layers/z/scenarios/a"
// or in the form of "layers/z/scenarios/a"
func (d *Diagram) GetBoard(boardPath string) *Diagram {
path := strings.Split(boardPath, string(os.PathSeparator))
if len(path) == 0 || len(boardPath) == 0 {
return d
}

return d.getBoard(path)
}

func (d *Diagram) getBoard(boardPath []string) *Diagram {
head := boardPath[0]

if head == "index" {
return d
}

switch head {
case "layers":
if len(boardPath) < 2 {
return nil
}
for _, b := range d.Layers {
if b.Name == boardPath[1] {
return b.getBoard(boardPath[2:])
}
}
case "scenarios":
if len(boardPath) < 2 {
return nil
}
for _, b := range d.Scenarios {
if b.Name == boardPath[1] {
return b.getBoard(boardPath[2:])
}
}
case "steps":
if len(boardPath) < 2 {
return nil
}
for _, b := range d.Steps {
if b.Name == boardPath[1] {
return b.getBoard(boardPath[2:])
}
}
}

for _, b := range d.Layers {
nhooyr marked this conversation as resolved.
Show resolved Hide resolved
if b.Name == head {
return b.getBoard(boardPath[2:])
}
}
for _, b := range d.Scenarios {
if b.Name == head {
return b.getBoard(boardPath[2:])
}
}
for _, b := range d.Steps {
if b.Name == head {
return b.getBoard(boardPath[2:])
}
}
return nil
}

func (diagram Diagram) Bytes() ([]byte, error) {
b1, err := json.Marshal(diagram.Shapes)
if err != nil {
Expand Down