Skip to content

fix(sandbox): Fix broken scripts on sandbox build - #1824

Merged
sampotts merged 5 commits into
videojs:mainfrom
spuppo-mux:fix/sandbox_scripts
Jul 24, 2026
Merged

fix(sandbox): Fix broken scripts on sandbox build#1824
sampotts merged 5 commits into
videojs:mainfrom
spuppo-mux:fix/sandbox_scripts

Conversation

@spuppo-mux

@spuppo-mux spuppo-mux commented Jul 13, 2026

Copy link
Copy Markdown
Collaborator

When running pnpm run build the following was failing

  1. On Sandbox: Unknown file extension ".ts"

    • vite.config.ts was doing a runtime await import(pathToFileURL('scripts/shared.ts')), which bypassed esbuild and was used Node's native loader. Switched to a static import { mirrorTemplatesToSrc } from './scripts/shared' (bundled/transpiled with the config).
  2. On Site: DurationFormat is not a constructor
    Intl.DurationFormat exists in Node 24 but not Node 22:

    • Added a fallback to format.ts: getDurationFormatter now falls back when Intl.DurationFormat is absent. Gated the two Intl-delegation tests with it.runIf and added a fallback-path test.
    • Per site/src/content/docs/concepts/browser-support.mdx:25:

    When we use a platform feature that hasn't landed in all supported browsers yet, we provide a fallback. The player works either way; the experience may differ slightly in older browsers. For example, tooltips and popovers use CSS anchor positioning where available and fall back to absolute positioning where it isn't.

This PR also includes a node version upgrade in .node-version and .npmrc, as well as switching the node-version to use node-version-file in two github workflows (as done in the rest of the workflows)

Open question (resolved):

Node version mismatch:
- 22.19.0: .node-version (read by fnm/asdf/nodenv/volta), .npmrc, .node-version, and package.json engines >=22.19.0.
- 24.14.0: .nvmrc (nvm only), plus most CI jobs via node-version-file.

Resolved: We resolved to upgrade node version to 24.14.0.


Note

Medium Risk
Time-display fallback changes user-visible strings on older runtimes; Node 24 bump affects all contributors and CI. Sandbox build path change is localized but on the critical build pipeline.

Overview
Fixes pnpm run build failures from sandbox Vite config loading and site builds on Node without Intl.DurationFormat, and aligns local/CI Node to 24.14.0.

Sandbox: vite.config.ts no longer runtime-imports scripts/shared.ts via pathToFileURL (which triggered Node’s native .ts loader). It now statically imports mirrorTemplatesToSrc from ./scripts/shared so the config is bundled/transpiled with the rest of the Vite setup.

Time formatting: getDurationFormatter in packages/utils/src/time/format.ts treats Intl.DurationFormat as optional. When it’s missing (e.g. Node < 23 during SSR/prerender), it uses createFallbackFormatter: exact digital H:MM:SS output and English comma-separated phrases for localized styles. Tests gate Intl-specific cases with it.runIf / it.skipIf and add loadWithFallback() coverage for the fallback path.

Node version: .node-version and .npmrc move from 22.19.0 to 24.14.0 (matching .nvmrc). bundle-size.yml and website-tests.yml switch setup-node from hardcoded node-version: 22 to node-version-file: '.nvmrc'.

Reviewed by Cursor Bugbot for commit 97ee7df. Bugbot is set up for automated code reviews on this repo. Configure here.

@spuppo-mux
spuppo-mux requested a review from sampotts July 13, 2026 18:32
@vercel

vercel Bot commented Jul 13, 2026

Copy link
Copy Markdown

@spuppo-mux is attempting to deploy a commit to the Mux Team on Vercel.

A member of the Team first needs to authorize it.

@netlify

netlify Bot commented Jul 13, 2026

Copy link
Copy Markdown

Deploy Preview for vjs10-site ready!

Name Link
🔨 Latest commit 97ee7df
🔍 Latest deploy log https://app.netlify.com/projects/vjs10-site/deploys/6a569cfb60c2dd0008853538
😎 Deploy Preview https://deploy-preview-1824--vjs10-site.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

Comment thread packages/utils/src/time/tests/format.test.ts
@sampotts

Copy link
Copy Markdown
Collaborator

As per Slack, let's normalize to 24.18.0 (latest 24 which is LTS) and perhaps add a test for the fallback.

uses: actions/setup-node@v5
with:
node-version: 22
node-version-file: '.nvmrc'

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@mihar-22 double checking with you if these were hardcoded to 22 on purpose or if these changes are ok

@spuppo-mux

Copy link
Copy Markdown
Collaborator Author

note: only places I didn't update node was package.json > engines.node assuming we still want to support lower node versions and the corresponding mention in CONTRIBUTING.md.

question: Related to the previous note, should I also update @types/node version in devDependencies to ^22 | ^24?

@spuppo-mux
spuppo-mux requested review from mihar-22 and sampotts July 14, 2026 20:47
@vercel

vercel Bot commented Jul 24, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
v10-sandbox Ready Ready Preview, Comment Jul 24, 2026 9:45pm

Request Review

@sampotts
sampotts merged commit 20f118e into videojs:main Jul 24, 2026
23 of 25 checks passed
@luwes luwes mentioned this pull request Jul 22, 2026
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