build: add deploy preview of the Vega editor #3925

hydrosquall · 2024-05-19T20:17:43Z

Motivation

Fix developer-experience: deployment previews for pull requests #3924

Changes

Set explicit yarn and node versions so that the build step can succeed in Cloudflare CI
Move installing data out of the postinstall as it was causing the initial install to break. I moved it to a docs building step
Add a build script for setting up a site deploy preview

Testing

Visit the preview URL: https://cameron-yick-deploy-previews.vega-628.pages.dev/
Check that console logs from both vega and a subpackage like vega-view are both visible.
- (I'll remove these statements before merging)

cloudflare-pages · 2024-05-19T20:24:12Z

Deploying vega with Cloudflare Pages

Latest commit:	`bd6d74e`
Status:	✅ Deploy successful!
Preview URL:	https://654eaa71.vega-628.pages.dev
Branch Preview URL:	https://cameron-yick-deploy-previews.vega-628.pages.dev

View logs

hydrosquall · 2024-05-19T20:33:21Z

package.json

+  "workspaces": [
+    "packages/*"
+  ],
+  "engines": {


hydrosquall · 2024-05-19T20:35:23Z

package.json

+  "engines": {
+    "node": ">=18.18.0"
+  },
+  "packageManager": "yarn@1.22.19"


Without this, we get an auto-upgrade to yarn 3.

16:26:44.651 | Detected the following tools from environment: yarn@3.6.3, nodejs@18.17.1 -- | -- 16:26:44.651 | Installing project dependencies: yarn 16:26:45.212 | ➤ YN0070: Migrating from Yarn 1; automatically enabling the compatibility node-modules linker 👍 16:26:45.212 | 16:26:45.342 | ➤ YN0000: ┌ Resolution step 16:26:46.174 | ➤ YN0032: │ canvas@npm:2.11.2: Implicit dependencies on node-gyp are discouraged 16:26:46.421 | ➤ YN0032: │ nan@npm:2.17.0: Implicit dependencies on node-gyp are discouraged 16:26:47.253 | ➤ YN0032: │ fsevents@npm:2.3.2: Implicit dependencies on node-gyp are discouraged 16:26:49.121 | ➤ YN0061: │ @npmcli/move-file@npm:2.0.1 is deprecated: This functionality has been moved to @npmcli/fs 16:26:49.642 | ➤ YN0061: │ request@npm:2.88.2 is deprecated: request has been deprecated, see https://github.com/request/request/issues/3142 16:26:49.660 | ➤ YN0061: │ uuid@npm:3.4.0 is deprecated: Please upgrade to version 7 or higher. Older versions may use Math.random() in certain circumstances, which is known to be problematic. See https://v8.dev/blog/math-random for details. 16:26:49.815 | ➤ YN0061: │ har-validator@npm:5.1.5 is deprecated: this library is no longer supported 16:26:50.972 | ➤ YN0032: │ @parcel/watcher@npm:2.0.4: Implicit dependencies on node-gyp are discouraged 16:26:51.046 | ➤ YN0032: │ node-addon-api@npm:3.2.1: Implicit dependencies on node-gyp are discouraged 16:27:01.937 | ➤ YN0002: │ @nrwl/devkit@npm:16.10.0 doesn't provide nx (p048f2), requested by @nx/devkit 16:27:01.937 | ➤ YN0060: │ root-workspace-0b6124@workspace:. provides eslint (p1305c) with version 8.53.0, which doesn't satisfy what typescript-eslint and some of its descendants request 16:27:01.938 | ➤ YN0000: │ Some peer dependencies are incorrectly met; run yarn explain peer-requirements <hash> for details, where <hash> is the six-letter p-prefixed code 16:27:01.942 | ➤ YN0000: └ Completed in 16s 599ms 16:27:02.015 | ➤ YN0000: ┌ Post-resolution validation 16:27:02.016 | ➤ YN0028: │ The lockfile would have been modified by this install, which is explicitly forbidden.

hydrosquall · 2024-05-22T02:39:28Z

package.json

    "license": "lerna exec -- cp ../../LICENSE .",
    "release": "yarn license && lerna publish from-package",
    "serve": "serve packages/vega/ -l 8080",
    "test": "yarn test:no-lint && yarn lint",
    "test:no-lint": "lerna run test --ignore vega-typings && lerna run test --scope vega-typings",
    "lint": "ESLINT_USE_FLAT_CONFIG=true yarn eslint -c eslint.config.mjs .",
-    "format": "yarn lint --fix",
-    "postinstall": "yarn data"


If we must keep this as a postinstall step and can't move this to docsbuild, an alternate approach would be to replace this with a guard so that I can disable the postinstall step with an env variable.

Convinced myself it would be less of a change for existing devs to take this route.

d4bacf0

hydrosquall · 2024-05-22T02:43:50Z

package.json

+    "node": ">=18.18.0"
+  },
+  "packageManager": "yarn@1.22.19",
+  "volta": {


Recommended node version manager - can drop this if it's distracting / people prefer to bring their own node version management solution

https://volta.sh/

As it stands, you won't be able to build the project if you have an older version (like node 18.15) running locally.

domoritz

Remove logs of course.

domoritz · 2024-05-22T16:16:53Z

package.json

+    "node": ">=18.18.0"
+  },
+  "packageManager": "yarn@1.22.19",
+  "volta": {


Let's remove this. I use Volta but not everyone does and I like to use node 20+ since 18 is pretty old now.

domoritz · 2024-05-22T16:17:41Z

scripts/build-editor-preview.sh

+echo "Installing Vega"
+
+# Link every package to make sure the editor uses the local version
+for package in packages/*; do


Can't lerna do this?

Lerna can handle running a command in every package, but for linking it delegates to yarn / npm https://lerna.js.org/docs/legacy-package-management#migrating-from-lerna-bootstrap-lerna-add-and-lerna-link-in-lerna-v7-and-later

It's supposed to prefer the local version if that packages the package version but the bug we're currently facing in vega is that multiple versions of the subpackages are active at once

The yarn monorepo solution for this is the workspaces:* protocol so we can make the package prefer the version in this monorepo, the specific version number would be replaced on the publish step. To use it we would have to upgrade to yarn v2 and up.

I think that move is worth it, but didn't want to get that tied up in the scope of this PR.

domoritz · 2024-05-22T16:18:23Z

scripts/build-editor-preview.sh

+cd editor
+yarn --frozen-lockfile --ignore-scripts
+
+# Make sure we prefer the local version to the one from npm


This seems like a hack. What goes wrong if we didn't have this?

I'd agree this is a hack, ideally we shouldn't need to install these from NPM

Ideally we can remove this after we run through and make sure there's only 1 version of each package ( @lsh mentioned he might get to look into it in the vega slack , I didn't open a new issue for it )

domoritz · 2024-05-22T16:18:58Z

scripts/postinstall.sh

+set -eo pipefail
+
+# Run this script as long as the env variable
+# DISABLE_POSTINSTALL_SCRIPT is not set.


Where is this set?

In the CI dashboard config: https://dash.cloudflare.com//pages/view/vega/settings/environment-variables

build: add deploy preview for the vega editor

a0440c2

hydrosquall changed the title ~~build: add deploy preview for the vega editor~~ build: add deploy preview of the Vega editor May 19, 2024

hydrosquall force-pushed the cameron.yick/deploy-previews-vega branch from e4e9ddb to cfb8f33 Compare May 19, 2024 20:22

test: check if deployment console msg gets shipped

8aeb370

hydrosquall force-pushed the cameron.yick/deploy-previews-vega branch from cfb8f33 to 8aeb370 Compare May 19, 2024 20:24

hydrosquall force-pushed the cameron.yick/deploy-previews-vega branch from 5429e38 to 4858d26 Compare May 19, 2024 20:28

build: specify last yarn 1 version to avoid auto upgrade

2ea19fd

hydrosquall force-pushed the cameron.yick/deploy-previews-vega branch from 4858d26 to 2ea19fd Compare May 19, 2024 20:29

hydrosquall commented May 19, 2024

View reviewed changes

package.json

"workspaces": [

"packages/*"

],

"engines": {

Copy link

Member Author

hydrosquall May 19, 2024

Choose a reason for hiding this comment

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

hydrosquall commented May 19, 2024

View reviewed changes

build: force newer version of node for eslint compat

f63065f

hydrosquall force-pushed the cameron.yick/deploy-previews-vega branch 2 times, most recently from 99f2f6f to 550a7ee Compare May 19, 2024 20:51

test: try moving postinstall script to docs site build

5c08192

hydrosquall force-pushed the cameron.yick/deploy-previews-vega branch from 550a7ee to 5c08192 Compare May 19, 2024 20:56

hydrosquall added 5 commits May 19, 2024 17:01

build: check if sub package works without explicit link ref

2401ebc

test: bump workspace version

3590446

enable lerna workspace

dc00097

build: upgrade baseline node version to v18.18.0

815a0d7

move force view build

7a8c8f0

hydrosquall force-pushed the cameron.yick/deploy-previews-vega branch from 86866e9 to 7a8c8f0 Compare May 19, 2024 21:27

link the packages before building them

ae5c73c

hydrosquall force-pushed the cameron.yick/deploy-previews-vega branch from 0d17c99 to ae5c73c Compare May 19, 2024 21:42

hydrosquall added 2 commits May 19, 2024 17:47

yarn link everything

5b8a5b3

fix comments in build script

6f03252

hydrosquall commented May 22, 2024

View reviewed changes

hydrosquall force-pushed the cameron.yick/deploy-previews-vega branch from f2886e1 to d4bacf0 Compare May 22, 2024 02:46

feat: bring back postinstall script, but let CI disable it

9c7fb3f

hydrosquall force-pushed the cameron.yick/deploy-previews-vega branch from d4bacf0 to 9c7fb3f Compare May 22, 2024 02:48

hydrosquall self-assigned this May 22, 2024

hydrosquall marked this pull request as ready for review May 22, 2024 03:10

hydrosquall requested a review from a team as a code owner May 22, 2024 03:10

domoritz approved these changes May 22, 2024

View reviewed changes

hydrosquall added 2 commits May 23, 2024 22:07

fix: explain build linking hack + remove volta version pin

4ce2a49

fix: remove build test console logs

bd6d74e

hydrosquall merged commit 11a33fb into main May 24, 2024
5 checks passed

hydrosquall deleted the cameron.yick/deploy-previews-vega branch May 24, 2024 03:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

build: add deploy preview of the Vega editor #3925

build: add deploy preview of the Vega editor #3925

hydrosquall commented May 19, 2024 •

edited

cloudflare-pages bot commented May 19, 2024 •

edited

hydrosquall May 19, 2024

hydrosquall May 19, 2024 •

edited

hydrosquall May 22, 2024 •

edited

hydrosquall May 22, 2024

hydrosquall May 22, 2024

domoritz left a comment

domoritz May 22, 2024

domoritz May 22, 2024

hydrosquall May 24, 2024

domoritz May 22, 2024

hydrosquall May 24, 2024

domoritz May 22, 2024

hydrosquall May 24, 2024

build: add deploy preview of the Vega editor #3925

build: add deploy preview of the Vega editor #3925

Conversation

hydrosquall commented May 19, 2024 • edited

Motivation

Changes

Testing

cloudflare-pages bot commented May 19, 2024 • edited

Deploying vega with Cloudflare Pages

Choose a reason for hiding this comment

hydrosquall May 19, 2024 • edited

Choose a reason for hiding this comment

hydrosquall May 22, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

domoritz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hydrosquall commented May 19, 2024 •

edited

cloudflare-pages bot commented May 19, 2024 •

edited

hydrosquall May 19, 2024 •

edited

hydrosquall May 22, 2024 •

edited