Skip to content

Vite + fix canary and beta ci jobs#317

Open
NullVoxPopuli wants to merge 48 commits intomasterfrom
nvp/its-vite-time
Open

Vite + fix canary and beta ci jobs#317
NullVoxPopuli wants to merge 48 commits intomasterfrom
nvp/its-vite-time

Conversation

@NullVoxPopuli
Copy link
Copy Markdown
Contributor

@NullVoxPopuli NullVoxPopuli commented Apr 17, 2026

Marked as enhancement because there are opt-in imports for current-era strict-mode users that don't affect anyone else -- piggy backing off previously TS-only entrypoints


Converts the docs and test-app to vite using the vite codemod from mainmatter.

Also migrates from fastboot to vite-ember-ssr since fastboot doesn't work with vite.
(and vite-ember-ssr is quite a bit more ergonomic)

each app:

  1. Run codemod
  2. run lint:fix
  3. Remove v1 addons (fastboot, fastboot-testing, ember-cli-code-coverage, etc)
    2. ember-try replaced with @embroider/try, CI updated accordingly
    3. ember-cli-sass removed
    4. ember-cli-github-pages removed
  4. add vite-ember-ssr
    1. Migrate each vite app to minimal, shedding compat (required)
    2. type=module the apps

incidentally this supersedes:

  • Fix canary and beta ci jobs #315
    • normally getting canary and beta passing wouldn't require vite, but the docs on this project use SSG, so I had to remove fastboot and use vite-ember-ssr

prior for reference:

NullVoxPopuli and others added 2 commits April 18, 2026 18:03
…-continue-on-error

Set allow-failure on every try scenario
The min-supported, ember-lts-4.12, and ember-lts-5.12 try-scenarios all
set ENABLE_COMPAT_BUILD=true and install @embroider/compat + ember-auto-import
so they can exercise the classic Ember 4.x/5.12 resolution path. The compat
scenario was failing with:

    [vite]: Rollup failed to resolve import "ember-testing" from
    "test-app/tests/index.html?html-proxy&index=0.js".

`tests/index.html` has `<script type="module">import "ember-testing";</script>`
which is a bare specifier that needs the classic resolver. In the default
build, `ember()` is enough, but the compat path requires
`classicEmberSupport()` (see ef4/memory-scroll vite.config.mjs for the
reference pattern).

Spread `classicEmberSupport()` into plugins only when ENABLE_COMPAT_BUILD is
set, matching the pattern already used by the try scenarios themselves.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
NullVoxPopuli and others added 4 commits April 18, 2026 20:01
test-app/package.json has "type": "module", so an ember-cli-build.js file
would be parsed as ESM. The generated content is CommonJS (require /
module.exports), so the file needs a .cjs extension to be interpreted
correctly.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…classic-ember-support

Enable classicEmberSupport() for compat-build scenarios
The min-supported try-scenario was failing with:

    [vite]: Rollup failed to resolve import "@ember/enumerable/mutable"
    from ".../@embroider/legacy-inspector-support/src/modules-4-12.js"

`@embroider/legacy-inspector-support` ships three entry points sharded by
ember-source range:
  - ember-source-3.28  → Ember < 4.8   (uses legacy mutable-enumerable path)
  - ember-source-4.8   → Ember 4.8–4.11 (adds @ember/enumerable/mutable)
  - ember-source-4.12  → Ember >= 4.12 (also adds @ember/owner)

test-app/app/app.js was hard-coded to the 4.12 entry, so the min-supported
scenario (ember-source ~4.2.0) pulled in modules-4-12.js, which imports
`@ember/enumerable/mutable` — a module that doesn't exist in Ember 4.2.

Use macroCondition + dependencySatisfies + importSync from @embroider/macros
to pick the right entry at build time based on the installed ember-source.
The macros compile the untaken branches away, so each try scenario only
pulls in the import graph it can actually resolve.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
NullVoxPopuli and others added 2 commits April 18, 2026 19:25
…port-macro

Pick legacy-inspector-support entry by ember-source version
`min-supported` and `ember-lts-4.12` were failing in vite build with:

    [vite]: Rollup failed to resolve import "require" from
    ".../@ember/test-helpers/-internal/build-registry.js"

The line responsible is at the top of
`@ember/test-helpers@3.x/addon-test-support/@ember/test-helpers/-internal/build-registry.ts`:

    import require, { has } from 'require';

This is the AMD `require` module provided by classic Ember's loader.js.
When vite/rollup processes this file (after `@embroider/compat` rewrites
it into the addon's `rewritten-packages` tree), there is no `require`
module to resolve and the build fails.

`@ember/test-helpers@4.0.0` dropped that import and re-implemented
build-registry on top of `@ember/-internals/container` and
`./-owner-mixin-imports.js`, so it no longer depends on a loader.js-style
`require`. Its peer is `ember-source >= 4.0.0`, which satisfies both the
min-supported scenario (~4.2.0) and ember-lts-4.12. `ember-qunit@^8.0.0`
peers `@ember/test-helpers: >=3.0.3`, so the bump does not violate it.

Also bump `@ember/test-waiters` to `^3.1.0` to stay within the window
expected by test-helpers 4.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
NullVoxPopuli and others added 5 commits April 18, 2026 19:54
…pers-for-require

Bump @ember/test-helpers to ^4 in ember4 try scenarios
Two stacked runtime issues were blocking ember-lts-4.12 once the
`require` import issue from #321 cleared. Both only affect the compat
path (ENABLE_COMPAT_BUILD=true, ember4 deps overrides) and are hidden
for ember-source >= 6 because the upstream code was rewritten.

1. "Cannot read properties of undefined (reading 'has')" at
   `makeComputedDecorator` → `COMPUTED_GETTERS.has(...)`.

   ember-source <= 5.x's `@ember/-internals/metal` has
   `let COMPUTED_GETTERS; if (DEBUG) { COMPUTED_GETTERS = new WeakSet(); }`
   paired with `assert(..., !COMPUTED_GETTERS.has(...))`. `DEBUG` comes
   from `@glimmer/env`, whose published default is `DEBUG = false`.
   Without something resolving `DEBUG` at compile time, rollup tree-shakes
   the init but the `assert` predicate still runs eagerly, so
   `COMPUTED_GETTERS` is undefined at runtime.

   Conditionally add `babel-plugin-debug-macros` (only when
   ENABLE_COMPAT_BUILD is set) with `flags: [{ source: '@glimmer/env',
   flags: { DEBUG: true } }]` so the plugin inlines `DEBUG = true` in
   ember-source. The init and the use stay consistent. Non-compat
   scenarios don't need the plugin — modern ember-source uses
   `isDevelopingApp()` from `@embroider/macros`, which `buildMacros()`
   already handles.

2. ember-qunit@8 + @ember/test-helpers@4 rely on classic-Ember globals
   that don't exist in a vite build: `start()` auto-calls `loadTests()`
   which iterates `requirejs.entries`, and `visit` reads
   `global.EmberENV._APPLICATION_TEMPLATE_WRAPPER`. Both were removed in
   ember-qunit@9 and @ember/test-helpers@5 respectively. Bump those two
   pins in the ember4 deps stanza so the scenario exercises the same
   surface modern scenarios do:

       ember-qunit:          ^8.0.0  ->  ^9.0.0
       @ember/test-helpers:  ^4.0.0  ->  ^5.0.0

   Both peer `ember-source >= 4.0.0`, so Ember 4.12 is still within the
   supported range. ember-qunit@9 peers `@ember/test-helpers >=3.0.3`,
   so the test-helpers bump stays within bounds.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`min-supported` installs `ember-source: ~4.2.0` but the test-app's
default `@glimmer/component@2.1.1` has `import { setOwner } from
'@ember/owner'`. `@ember/owner` didn't ship as a module path until
ember-source 4.10, so rollup fails with:

    [vite]: Rollup failed to resolve import "@ember/owner" from
    ".../@glimmer/component@2.1.1/node_modules/@glimmer/component/dist/index.js"

Use `babel-plugin-ember-polyfill-get-and-set-owner-from-ember-owner` to
rewrite those imports back to `@ember/application` (which had `getOwner`
/`setOwner` in every 4.x release). The plugin only runs when
`ENABLE_COMPAT_BUILD` is set, next to babel-plugin-debug-macros. It's a
no-op for scenarios that still resolve `@ember/owner` natively
(ember-lts-5.12 and later) because `@ember/application` re-exports the
same symbols there.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…2-compat-runtime

Make ember-lts-4.12 compat scenario runnable
…-ember-owner-polyfill

Polyfill @ember/owner imports for min-supported scenario
@NullVoxPopuli NullVoxPopuli marked this pull request as ready for review April 19, 2026 03:14
@NullVoxPopuli NullVoxPopuli changed the title Vite Vite + fix canary and beta ci jobs Apr 19, 2026
NullVoxPopuli and others added 2 commits April 18, 2026 23:22
With #323 landed, `ember-lts-4.12` flips to:

    Uncaught ReferenceError: Cannot access 'getOwner' before initialization
    at app-*.js line N

ember-source >= 4.12's `@ember/application/index.js` re-exports the real
implementations from `@ember/owner` as aliases:

    import { getOwner as actualGetOwner, setOwner as actualSetOwner }
      from '@ember/owner';
    export const getOwner = actualGetOwner;
    export const setOwner = actualSetOwner;

`babel-plugin-ember-polyfill-get-and-set-owner-from-ember-owner` rewrote
the source of that import to `@ember/application` — turning the module
into a self-import that rollup then collapses into
`const getOwner = getOwner` (TDZ).

The plugin already tries to skip ember-source paths, but the guard only
short-circuits when the file is outside `.embroider/`; in a vite
compat build ember-source lives at
`node_modules/.embroider/rewritten-packages/ember-source.*/` so it
fell through and got rewritten. Gate the plugin through babel
`overrides` with a `test` filter that excludes any path containing
`/ember-source/`. The polyfill still runs where it needs to
(e.g. `@glimmer/component@2.1.1`'s `import { setOwner } from
'@ember/owner'`, which is how min-supported started passing in #323).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…olyfill-skip-ember-source

Don't run @ember/owner polyfill on ember-source itself
NullVoxPopuli-ai-agent pushed a commit to NullVoxPopuli-ai-agent/ember-polyfill-get-and-set-owner-from-ember-owner that referenced this pull request Apr 19, 2026
ember-source >= 4.12's `@ember/application/index.js` re-exports
`getOwner` and `setOwner` by importing them from `@ember/owner`:

    import { getOwner as actualGetOwner, setOwner as actualSetOwner }
      from '@ember/owner';
    export const getOwner = actualGetOwner;
    export const setOwner = actualSetOwner;

The existing filename guard only skipped ember-source files that were
outside `/.embroider/`. In an @embroider/compat build ember-source is
rewritten into `node_modules/.embroider/rewritten-packages/ember-source.HASH/
node_modules/ember-source/…`, so the path contains both `/ember-source/`
and `/.embroider/` — the guard fell through and we rewrote the import.
Rollup then collapsed the module into a self-import
(`const getOwner = getOwner`), which trips TDZ at load time with:

    ReferenceError: Cannot access 'getOwner' before initialization

Seen downstream in ember-cli/ember-page-title#317 on the
ember-lts-4.12 try scenario.

Drop the `.embroider` escape hatch and always skip files whose path is
inside an ember-source package. The rewrite still runs on user code
and third-party addons (e.g. `@glimmer/component@2.x`'s
`import { setOwner } from '@ember/owner'`) regardless of whether they
live directly in `node_modules/` or have been copied under
`.embroider/rewritten-packages/`.

Add tests that cover:
  - ember-source files in plain node_modules (unchanged behaviour)
  - ember-source files in .embroider/rewritten-packages (the bug)
  - third-party files in .embroider/rewritten-packages (still rewritten)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
NullVoxPopuli-ai-agent pushed a commit to NullVoxPopuli-ai-agent/ember-polyfill-get-and-set-owner-from-ember-owner that referenced this pull request Apr 19, 2026
ember-source >= 4.12's `@ember/application/index.js` re-exports
`getOwner` and `setOwner` by importing them from `@ember/owner`:

    import { getOwner as actualGetOwner, setOwner as actualSetOwner }
      from '@ember/owner';
    export const getOwner = actualGetOwner;
    export const setOwner = actualSetOwner;

The existing filename guard only skipped ember-source files that were
outside `/.embroider/`. In an @embroider/compat build ember-source is
rewritten into `node_modules/.embroider/rewritten-packages/ember-source.HASH/
node_modules/ember-source/…`, so the path contains both `/ember-source/`
and `/.embroider/` — the guard fell through and we rewrote the import.
Rollup then collapsed the module into a self-import
(`const getOwner = getOwner`), which trips TDZ at load time with:

    ReferenceError: Cannot access 'getOwner' before initialization

Seen downstream in ember-cli/ember-page-title#317 on the
ember-lts-4.12 try scenario.

Drop the `.embroider` escape hatch and always skip files whose path is
inside an ember-source package. The rewrite still runs on user code
and third-party addons (e.g. `@glimmer/component@2.x`'s
`import { setOwner } from '@ember/owner'`) regardless of whether they
live directly in `node_modules/` or have been copied under
`.embroider/rewritten-packages/`.

Add tests that cover:
  - ember-source files in plain node_modules (unchanged behaviour)
  - ember-source files in .embroider/rewritten-packages (the bug)
  - third-party files in .embroider/rewritten-packages (still rewritten)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@evoactivity evoactivity left a comment

Choose a reason for hiding this comment

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

Just some nitpicky things

Comment thread docs/app/app-ssr.js Outdated
Comment on lines +19 to +27
Object.assign(app, {
visit: async (...args) => {
const instance = await originalVisit(...args);

await settled();

return instance;
},
});
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This shouldn't be necessary, keep this simple and just return the SsrApp.create.

(I have an idea about this that should make doing this as a user unnecessary)

@@ -2,7 +2,7 @@ import type { TemplateOnlyComponent } from '@ember/component/template-only';
import { pageTitle } from 'ember-page-title';
import { Input } from '@ember/component';
import highlight from 'docs/helpers/highlight';
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should add file extensions to everything we import if adding to window-pane.gts

Comment thread docs/app/styles/app.scss
Comment on lines 40 to +43
-webkit-appearance: textfield;
-moz-appearance: textfield;
-ms-appearance: textfield;
appearance: textfield;
-moz-appearance: textfield;
-ms-appearance: textfield;
appearance: textfield;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

shouldn't need the prefixes anymore

Comment thread docs/vite.config.mjs Outdated
Comment thread docs/app/styles/app.scss
display: -webkit-flex;
display: flex;
display: flex;
display: flex;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Don't need all of these, just one display: flex.

Comment thread docs/vite.config.mjs Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants