Skip to content

Migrate SPA to Vite; adopt Prettier - #176

Merged
skearnes merged 13 commits into
mainfrom
vite-migration
May 13, 2026
Merged

Migrate SPA to Vite; adopt Prettier#176
skearnes merged 13 commits into
mainfrom
vite-migration

Conversation

@skearnes

@skearnes skearnes commented May 12, 2026

Copy link
Copy Markdown
Member

Summary

Modernizes the Vue SPA's tooling and fixes a few latent bugs that surfaced once it was actually exercised locally. Five threads:

  1. Build: @vue/cli-serviceVite 6. vue-cli is in maintenance mode and pinned by transitive deps that will never be patched. Vite is the official Vue successor.
  2. Format: clang-formatPrettier. clang-format treats .vue files as C-like and produces nonsense (which is what broke CI in the first place). Prettier is what ord-app already uses — adopting it here keeps formatting consistent between the two SPAs.
  3. Sass module system: @import@use ... as *. Every build was warning that @import will be removed in Dart Sass 3.0.0.
  4. Lint: eslint@7eslint@9 with flat config. The eslint 7→8 bump in the Vite commit cleared most of the npm churn but kept six deprecation warnings on every npm install; the v9 bump wipes all six.
  5. Latent bugs surfaced by local testing: Ketcher view rendering, two /search hangs (empty query and form-submit-while-mounted), buggy reactionIds normalization, and a dead no-console/no-debugger lint rule.

Once this lands, the overrides workaround proposed in #175 becomes dead code and that PR is superseded (already closed).

Result

Before After
npm packages installed 1037 269
Open dependabot alerts 23 0
npm audit 23 issues 0 vulnerabilities
npm install deprecation warnings 6 (eslint 7 → rimraf/inflight/glob/@humanwhocodes chain) 0
npm run build time (local) ~16s ~5s
JS formatter clang-format (broken on .vue) Prettier (matches ord-app)
Sass deprecation warnings many per build 0
/ketcher page renders but unstyled/broken renders correctly
/search with no filters hangs forever (stuck spinner) shows a "configure filters" prompt
/search from filter panel submit hangs forever; only worked after refresh submits + polls correctly without remount
Selected-set view with no ?reaction_id= "Download" button enabled; POSTs [undefined] button disabled; clean empty state
no-console/no-debugger rules dead (NODE_ENV never set) warn in CI

Commits in this PR

Commit What
d5d1c4d Vite migration: swap @vue/cli-service + @vue/cli-plugin-* for vite ^6.4.2 + @vitejs/plugin-vue + vite-plugin-static-copy; move public/index.htmlapp/index.html; process.env.BASE_URLimport.meta.env.BASE_URL; rename .eslintrc.js.eslintrc.cjs; drop babel.config.js, vue.config.js, core-js, @babel/eslint-parser; bump eslint 7 → 8.57, eslint-plugin-vue 8 → 9
16dcdac CI: replace check_javascript's apt-install + find -exec clang-format step with npm run format:check; add app/.prettierrc.json (verbatim copy of ord-app/ui/.prettierrc.json) and app/.prettierignore; add prettier devDep + format / format:check scripts
2ed25e2 One-time prettier --write . pass over app/ — 47 files, no behavioral change (whitespace, quote style, trailing commas, Vue attribute layout)
b98abde Add a local prettier --check hook to .pre-commit-config.yaml, mirroring the existing ty-check setup
26c74b0 Migrate 36 @import lines across 24 files to @use '...' as *; clears all Dart Sass 3.0.0 deprecation warnings. Plain CSS @import url() and CSS-file @import are left as-is. as * keeps every $lightgrey/$medgrey/$linkblue reference in the global namespace, so no $varname references needed to change
510d505 Bug fix: drop scoped from MainKetcher.vue's <style> block. Vue scoped CSS appends [data-v-HASH] to every selector, but Ketcher builds its DOM at runtime via JS and those elements don't carry the scope attribute — so all ~1300 Ketcher CSS rules failed to match anything. Also drops two empty button selectors in SearchResults.vue files that were producing per-build warnings
70b281e Bug fix: short-circuit /search when there are no URL query params. The watch on $route.query was firing /api/submit_query with an empty string on mount, the backend rejects that with ValueError, and the SPA was polling forever. Frontend now bails early and the template shows a "configure the filters" prompt instead
8de311e README: combine the cleanup-pass refresh (project layout fix, setup.pyuv, ## Deployment pointer to ord-infrastructure) with the local-stack fixes I needed to actually bring this up (redis prerequisite, uv run uvicorn … instead of fastapi dev, conda-env-activation note for initdb discovery, macOS AirPlay port-5000 collision, Ketcher extraction step clarified, Vite script aliases)
04f4016 Bump eslint 8.57 → 9.20 and migrate to flat config. The eslint 8 dep tree pulled in eslint@8.57.1, @humanwhocodes/{config-array,object-schema}, rimraf@3, glob@7, and inflight@1 — all six packages produce deprecation warnings during npm install. v9 wipes the entire chain. Delete .eslintrc.cjs and .eslintignore; add eslint.config.js (uses vue.configs['flat/essential'] from eslint-plugin-vue@9.33)
1cac486 Bug fix surfaced by ESLint 9's no-constant-binary-expression rule: reactionIds in MainSelectedSet.vue was `[query.reaction_id]
c9534dd Self-review follow-ups. (a) Drop the resolve.extensions workaround from vite.config.js and add .vue to all 50 extension-less imports across 16 files — Vite officially discourages relying on implicit .vue resolution. (b) Pin the dev-server proxy target to 127.0.0.1:5000 (was 0.0.0.0:5000) so the macOS AirPlay-on-port-5000 collision is dodged deterministically. (c) Fix the always-false process.env.NODE_ENV === 'production' check in eslint.config.js — ESLint never sets NODE_ENV, so no-console/no-debugger were never warning. Switch to process.env.CI which GitHub Actions sets; verified by running lint both locally (7 warnings) and as CI=true npm run lint (17 warnings — the 10 extra are previously-silent console statements)
4f6c656 Bug fix: /search was hanging until refresh when the user submitted the filter form. updateSearchOptions navigates via $router.push({name: 'search', query: …}) which reuses the mounted component, so mounted() does NOT re-fire — only the watch on $route.query does. The polling-setup branch for 202 (task pending) responses lived only inside mounted(), so the watch path hit the first 202, had no branch for it, and stuck the spinner. Pull "fetch then poll if 202" out of mounted into runSearch() called from both paths. Also fix two adjacent bugs: clearInterval(...) was never nulling searchPollingInterval (subsequent searches saw a stale truthy handle and skipped polling), and a new query could inherit the previous query's searchTaskId

Preserved behavior

  • Dev server still binds port 8080 (matches the old vue-cli default).
  • npm run serve still works; added npm run dev and npm run preview as Vite-conventional aliases.
  • @/* import alias preserved in vite.config.js + jsconfig.json.
  • All .vue imports now carry explicit extensions, so Vite's default resolver handles everything without resolve.extensions overrides.

Infrastructure: zero changes

The Dockerfile is unchanged — it still calls npm install && npm run build and copies dist/ to nginx's web root. nginx's SPA try_files $uri $uri/ /index.html config doesn't care about asset paths, so Vite's assets/ directory works in place of vue-cli's js/+css/.

Test plan

  • cd app && rm -rf node_modules package-lock.json && npm install produces 269 packages, zero deprecation warnings, zero vulnerabilities
  • cd app && npm run build succeeds in ~5s with no Dart Sass deprecation warnings and no Vite warnings; dist/ contains index.html, assets/*, templates/* (3 ketcher files), favicon.ico, img/
  • cd app && npm run serve boots at http://localhost:8080/ in ~150ms; / returns 200; /src/main.js is served and transformed
  • cd app && npm run preview serves the production bundle; /templates/library.svg (2.5MB) and /templates/fg.sdf (30KB) are reachable
  • cd app && npm run format:check reports "All matched files use Prettier code style!"
  • cd app && npm run lint passes (7 unused-var warnings, no errors); CI=true npm run lint correctly adds 10 console-statement warnings
  • pre-commit run prettier --all-files passes
  • End-to-end against a live FastAPI backend (ORD_INTERFACE_TESTING=TRUE uv run uvicorn …) + redis: / renders, /browse lists the 3 test datasets, /search shows the new "configure filters" prompt instead of hanging, submitting a query from the filter panel resolves without a refresh, /ketcher renders correctly (CSS no longer scoped away)
  • CI Checks workflow (check_javascript running npm run format:check) is green
  • CI Tests > test_app workflow builds the Docker image (which calls npm run build inside the container) and runs the puppeteer smoke tests

Follow-ups (out of scope for this PR)

  • Stand up Vitest so the SPA bug fixes (Ketcher scope, empty-search short-circuit, reactionIds normalization, runSearch polling fix) get unit-test coverage instead of relying on manual verification.
  • Decide whether to scope Ketcher's CSS to its own route lifecycle (currently it loads globally on first /ketcher visit and persists for the SPA's lifetime; Ketcher uses :where()-prefixed selectors with zero specificity so the practical risk is low, but it's not ideal).

🤖 Generated with Claude Code

@vue/cli is in maintenance mode and pinned by transitive dependencies
that won't ever be patched without a major-version downgrade of the
toolchain. Vite is the official Vue successor and resolves the entire
class of npm advisories the SPA has been carrying.

Toolchain changes:
- Replace @vue/cli-service ~5.0.0 and friends with vite ^6.4.2 +
  @vitejs/plugin-vue. Drops 768 transitive packages from npm install.
- Replace the chainWebpack copy step in vue.config.js (ketcher
  templates → dist/templates) with vite-plugin-static-copy. Same
  output layout.
- Move public/index.html to app/index.html (Vite serves index.html
  from the project root) and add the explicit
  `<script type="module" src="/src/main.js">` tag that Vite expects.
- Replace process.env.BASE_URL with import.meta.env.BASE_URL in
  router/index.js (the only env reference in src/).
- Rename .eslintrc.js → .eslintrc.cjs because package.json now sets
  "type": "module"; drop the @babel/eslint-parser parser (espree
  handles ES2022 fine for the source style here).
- Drop babel.config.js (no longer used) and core-js (vite/esbuild
  honors browserslist without auto-polyfilling).
- Bump eslint 7 → 8.57 and eslint-plugin-vue 8 → 9.

Configuration choices preserved:
- Dev server still binds port 8080 (matches old vue-cli default).
- `npm run serve` still works as before; added `npm run dev` and
  `npm run preview` as Vite-conventional aliases.
- `@/*` import alias preserved in vite.config.js + jsconfig.json.
- Vite's resolver is configured with the same implicit extensions
  the old webpack resolver allowed, so existing extension-less
  imports throughout the codebase keep working without churn.

Verified locally:
- `npm install` produces 269 packages, 0 vulnerabilities (was 1037
  packages, 23 dependabot alerts).
- `npm run build` produces a working `dist/` (index.html +
  assets/* + templates/* + favicon + img/). Build time ~5s vs
  ~16s with vue-cli.
- `npm run serve` boots at http://localhost:8080/ in 156ms;
  / returns 200 and /src/main.js is transformed correctly.
- `npm run preview` (production preview) serves dist/, including
  /templates/library.svg (2.5MB) and /templates/fg.sdf (30KB).
- `npm run lint` passes (one pre-existing unused-var warning in
  src/utils/amount.js).

Dockerfile is unchanged: it still calls `npm install && npm run
build` and copies `dist/` to nginx's web root. nginx's SPA
`try_files` config doesn't care about asset paths, so the new
`assets/` directory works in place of vue-cli's `js/`+`css/`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
skearnes and others added 3 commits May 12, 2026 19:25
clang-format treats .vue files as C-like and produces nonsense
results. Prettier is the standard JS/TS/Vue formatter and is already
what ord-app uses, so adopting it here keeps formatting consistent
between the two repos.

- .github/workflows/checks.yml: drop the apt-install + `find ... -exec
  clang-format` step; add a setup-node step and run `npm run
  format:check` instead.
- app/package.json: add `prettier` as a dev dep with `format` and
  `format:check` scripts mirroring ord-app's setup.
- app/.prettierrc.json: copied verbatim from
  open-reaction-database/ord-app/ui/.prettierrc.json so the two SPAs
  produce identical output.
- app/.prettierignore: skip generated/vendored trees (dist/,
  node_modules/, public/, src/ketcher/, package-lock.json).

This commit only wires up the tool; the one-time `prettier --write`
pass follows in the next commit so the actual formatting churn is
trivially separable.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Applies the prettier config landed in the previous commit to the
existing SPA tree. No behavioral change — only whitespace, quote
style, trailing commas, and Vue attribute layout. Verified afterward:

- `npm run format:check` reports "All matched files use Prettier
  code style".
- `npm run build` still produces a working `dist/` in ~5s.
- `npm run lint` reports only the pre-existing unused-var warning
  in src/utils/amount.js.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Mirrors the ruff-check/ruff-format setup we have for Python: catches
formatting issues before they make it to CI. Implemented as a local
hook running `npx --no-install prettier --check .` inside app/, so
the hook version is whatever package-lock.json pins — no separate
mirrored repo to keep in sync.

Verified locally with `pre-commit run prettier --all-files`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@skearnes skearnes changed the title Migrate SPA from @vue/cli to Vite Migrate SPA to Vite; adopt Prettier May 12, 2026
skearnes and others added 4 commits May 12, 2026 19:33
Sass @import is deprecated and will be removed in Dart Sass 3.0.0.
Every build was emitting warnings like:

    DEPRECATION WARNING [import]: Sass @import rules are deprecated
    and will be removed in Dart Sass 3.0.0.

Mechanical migration of the 36 affected lines across 24 files:

    @import '@/styles/vars'        →  @use '@/styles/vars' as *
    @import '@/styles/vars.sass'   →  @use '@/styles/vars' as *
    @import "@/styles/vars"        →  @use '@/styles/vars' as *
    @import "./vars"               →  @use './vars' as *
    @import @/styles/vars          →  @use '@/styles/vars' as *
    @import "../../styles/vars"    →  @use '../../styles/vars' as *

Plain-CSS @imports (Google Fonts url() in App.vue and the Ketcher
.css in MainKetcher.vue) are left as-is — Sass treats those as CSS
pass-through and doesn't deprecate them.

The `as *` clause keeps every existing variable (`$lightgrey`,
`$medgrey`, `$linkblue`, etc.) in the global namespace, so no
$varname references in the consuming files need to change.

One nit fix: in App.vue the `@use` was placed after an
`@import url('...')` line, and Sass requires `@use` to be the first
rule in a file. Swapped the two lines.

Verified: `npm run build` produces a working dist/ in ~5s with
zero sass deprecation warnings; format:check and lint stay clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two small bug fixes surfaced while testing the Vite build:

1. MainKetcher.vue had `<style lang="sass" scoped>` wrapping the
   Ketcher CSS import. Vue's scoped CSS appends `[data-v-HASH]` to
   every selector — for Ketcher that's ~1300 rules — but Ketcher
   builds its own DOM inside `#root` at runtime via JavaScript and
   those dynamically-created elements don't carry the scope
   attribute, so none of the ~1300 rules match anything Ketcher
   actually renders. Result: Ketcher loads but looks "really weird
   and not usable". Drop `scoped` and add a comment explaining why
   it has to stay off.

2. SearchResults.vue (both search/ and dataset-view/ flavors) had
   a dangling `button` selector with no rules under it at the end of
   `.action-button-holder`. Sass warns about this on every build:
   "This selector doesn't have any properties and won't be
   rendered." Just drop the empty line.

Verified locally: clean `npm run build` (no warnings), prettier and
lint stay clean, and Ketcher CSS no longer carries
`[data-v-*]` attributes in the dev-server output (`grep -c
data-v-469446ff` went from 1325 → 0).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The Vue component was firing `/api/submit_query` with an empty query
string on first load (the watch on $route.query fires in mounted()
regardless of params). The backend rejects that with
`ValueError: No query parameters were specified.`, and the frontend
then polls forever waiting for a result that's never going to come —
manifesting as a stuck spinner on /search.

The original codebase had a `RandomSampleQuery(100)` fallback in the
old Flask client (commit 6886b0a) that masked this on the backend;
the FastAPI rewrite removed it intentionally, so the frontend is now
the right place to handle the empty-filter state.

Changes:
- getSearchResults() bails early if window.location.search is empty:
  set searchResults=[], loading=false, return without hitting the API.
- Template adds a separate empty-state message for the "no query yet"
  case ("Configure the filters on the left to search the database.")
  vs. the "search ran, nothing matched" case (existing message).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adopts the README rewrite from PR #174 (project layout corrected,
"How to Deploy" placeholder dropped, deployment pointer added,
schema-update instructions rewritten in terms of uv) and updates the
local-development section to match what actually works on a fresh
checkout:

- Add `redis` to the prerequisites list. The API uses redis for
  search-task state; if it isn't running, the search route hangs.
- Replace the broken `fastapi dev main.py --port=5000` invocation
  with `uv run uvicorn ord_interface.api.main:app --port 5000
  --reload`. `fastapi dev` requires the `fastapi[standard]` extra
  which isn't in our deps.
- Add a `redis-server --daemonize yes` step to the Local option.
- Note that the conda env from step 1 must stay activated so the
  testing.postgresql shells can find initdb on PATH.
- Flag the macOS port-5000 collision with AirPlay Receiver and
  document the workaround (bind/visit via 127.0.0.1, or disable the
  service in System Settings).
- Clarify the Ketcher extraction step so users end up with the right
  src/ketcher/static + templates layout the build expects.
- Mention `npm run dev` / `npm run preview` aliases now that the SPA
  is on Vite.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@skearnes
skearnes requested a review from bdeadman May 12, 2026 23:57
skearnes and others added 4 commits May 12, 2026 20:04
All six npm-install deprecation warnings on a fresh checkout chained
out of eslint@8.57.1:

    npm WARN deprecated rimraf@3.0.2 …
    npm WARN deprecated inflight@1.0.6 …
    npm WARN deprecated glob@7.2.3 …
    npm WARN deprecated @humanwhocodes/config-array@0.13.0 …
    npm WARN deprecated @humanwhocodes/object-schema@2.0.3 …
    npm WARN deprecated eslint@8.57.1 …

`npm ls` traced all of them to a single chain: `eslint@8` →
`file-entry-cache@6` → `flat-cache@3` → `rimraf@3` → `glob@7` →
`inflight@1`, plus eslint 8's own `@humanwhocodes/*` deps which were
renamed to `@eslint/*` in v9.

Migrate to ESLint 9 + flat config:

- Bump `eslint` ^8.57.1 → ^9.20.0 and add `@eslint/js` and `globals`
  as devDeps.
- Delete `.eslintrc.cjs` and `.eslintignore`; add `eslint.config.js`
  (flat config). `eslint-plugin-vue@9.33.0` exposes the Vue-3 essential
  ruleset at `vue.configs['flat/essential']`.
- Move the `ignores` list (`dist/`, `node_modules/`, `src/ketcher/`)
  into the flat config; ESLint 9 no longer reads `.eslintignore`.

One latent bug surfaced by ESLint 9's `no-constant-binary-expression`
rule, in MainSelectedSet.vue:

    return [this.$route.query.reaction_id] || [];

The array literal is always truthy, so `|| []` is dead code. Dropped
it; behavior is unchanged (the existing call site already always got
the LHS branch).

Verified locally: fresh `npm install` produces 269 packages with
**zero deprecation warnings** and **zero vulnerabilities**; lint,
format:check, and build all stay clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous commit dropped `|| []` because the LHS was always a
non-empty array literal (lint error). But the underlying intent was
correct: when `?reaction_id=…` is missing, the computed
`reactionIds` should be `[]` so the "Download Reaction Set" button
disables itself and the backend POST doesn't ship `[null]`.

vue-router gives back three different shapes for query params:

    ?<none>                       -> undefined
    ?reaction_id=foo              -> "foo"
    ?reaction_id=foo&reaction_id=bar -> ["foo", "bar"]

Normalize all three via `[value ?? []].flat()` so length reflects
the true count in every case.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three small follow-ups to the review:

1. Drop the `resolve.extensions` workaround in vite.config.js. The
   review pointed out Vite officially discourages including `.vue`
   in `extensions`, since implicit extensions can confuse IDE/TS
   tooling. Instead, do a one-shot pass adding `.vue` to all 50
   extension-less imports across 16 files in app/src; Vite's default
   resolver then handles everything cleanly without an opt-in.

2. Pin the dev-server proxy target to `127.0.0.1` instead of
   `0.0.0.0`. On macOS, `localhost`/`0.0.0.0` resolve via IPv6 first
   and hit Apple's AirPlay Receiver (which also listens on `*:5000`)
   instead of uvicorn (IPv4-only). Pinning to `127.0.0.1` dodges
   the ambiguity entirely. Also expand the Ketcher comment to note
   that the v2.5.1 build is self-contained — if Ketcher is ever
   updated to a chunked build, the static-copy targets need to grow.

3. Fix the always-false `NODE_ENV === 'production'` check in
   eslint.config.js. ESLint never sets NODE_ENV, so `no-console` and
   `no-debugger` were never warning. Switch to `process.env.CI`
   which GitHub Actions sets — verified: locally lints 7 warnings,
   CI mode lints 17 (the 10 extra are the previously-silent console
   statements).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Symptom: clicking "Search" from the filters panel left the spinner
running forever, but refreshing the page made the same query
complete quickly.

Root cause: the polling loop that handles `fetch_query_result`
returning 202 (task pending) was only set up inside `mounted()`.
When the user submits the filter form, `updateSearchOptions` calls
`this.$router.push({name: 'search', query: this.searchParams})`,
which navigates within the same component. Vue Router reuses the
mounted component, so `mounted()` does NOT re-fire — only the watch
on `$route.query` does. The watch was calling `getSearchResults()`
directly, missing the polling-setup branch. The first
`fetch_query_result` poll returned 202, the response handler had no
branch for that case, and the spinner stuck. Refresh worked because
the component remounts on a hard page load.

Fix: pull the "fetch then poll if 202" logic out of `mounted` into
`runSearch()`. Both `mounted` and the watch call `runSearch`, so the
polling-setup runs in both cases. Also:

- `stopPolling()` helper that clears the interval AND nulls
  `searchPollingInterval`. The previous code only called
  `clearInterval(...)`; the handle was never reset, so subsequent
  searches saw a stale truthy value and skipped polling setup.
- `runSearch` calls `stopPolling()` and resets `searchTaskId` at
  the top, so a new query doesn't poll for the previous query's
  stale task ID.
- Drop the dead `await … .then(…)` pattern in `mounted`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@skearnes
skearnes merged commit 5b44b39 into main May 13, 2026
15 checks passed
@skearnes
skearnes deleted the vite-migration branch May 13, 2026 21:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants