Open
Conversation
…ildl.js files are invalid for the current lint configuration
acc72bf to
fd16481
Compare
…-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>
3 tasks
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>
3 tasks
…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>
2 tasks
…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
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>
evoactivity
requested changes
Apr 20, 2026
Contributor
evoactivity
left a comment
There was a problem hiding this comment.
Just some nitpicky things
Comment on lines
+19
to
+27
| Object.assign(app, { | ||
| visit: async (...args) => { | ||
| const instance = await originalVisit(...args); | ||
|
|
||
| await settled(); | ||
|
|
||
| return instance; | ||
| }, | ||
| }); |
Contributor
There was a problem hiding this comment.
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'; | |||
Contributor
There was a problem hiding this comment.
Should add file extensions to everything we import if adding to window-pane.gts
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; |
Contributor
There was a problem hiding this comment.
shouldn't need the prefixes anymore
| display: -webkit-flex; | ||
| display: flex; | ||
| display: flex; | ||
| display: flex; |
Contributor
There was a problem hiding this comment.
Don't need all of these, just one display: flex.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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:
2. ember-try replaced with
@embroider/try, CI updated accordingly3. ember-cli-sass removed
4. ember-cli-github-pages removed
incidentally this supersedes:
prior for reference: