Rework Ember framework to use Vite #33048
Conversation
gossi
left a comment
There was a problem hiding this comment.
This is huge!!!
It's the foundation for integrating Ember with Storybook@10 through vite. Bringing all build systems together surely was a not-funny endeavor, so @mogstad did the herculean effort here.
This enables Ember for Storybook 10 in any form better than any previous implementation... and this is only the MVP. The more interessting work can start now.
Here is a list of things I see, we can do now (in no particular order):
- Typedoc integrations (args and slots in docs mode)
- Stories in gjs/gts (when you want to test/embed your components in HTML)
- Provide Decorators for Ember Devs (to support Embers DI system, or else components break 🙈)
- Mock services
- Set application (also globally, work for owner already done)
- Support CSF Next format for Ember
- Vitest integration
Basically, what Yann showed at Viteconf is now in reach for Ember: https://www.youtube.com/watch?v=YNQ_cJ6yy2Q
So, this PR unlocks the implementations I listed. We can do this in parallel now, thanks to the foundation of this PR.
PS. This started in september when I reached out to ask for help in integrating with Ember and took until today. Thanks a lot to @ndelangen and @JReinhold for providing their tremendeous support here, much appreciated.
But most credit goes to @mogstad 🥳 🎉 💯
| const { trackedObject } = await import('@ember/reactive/collections'); | ||
| const { renderComponent } = await import('@ember/renderer'); | ||
| const { destroy } = await import('@ember/destroyable'); |
There was a problem hiding this comment.
this might be cached? or does import() cache by itself?
There was a problem hiding this comment.
yes! import() does cache by itself, in both NodeJS and browsers.
|
View your CI Pipeline Execution ↗ for commit 02c8df6 ☁️ Nx Cloud last updated this comment at |
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 195 | 21 | 🎉 -174 🎉 |
| Self size | 15 KB | 38 KB | 🚨 +23 KB 🚨 |
| Dependency size | 28.75 MB | 28.30 MB | 🎉 -446 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
b80aeb6 to
935ae45
Compare
935ae45 to
256065b
Compare
📝 WalkthroughWalkthroughMigrates the Ember framework integration from Webpack5 to Vite, reorganizes client modules (client/*), implements new Ember render and source-generation logic, adds a Node CSF indexer, updates presets/types/exports, and adds new Ember template components and stories while removing legacy preview and Babel-preset artifacts. Changes
Sequence Diagram(s)mermaid mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Possibly related PRs
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (8)
code/frameworks/ember/tsconfig.json (1)
3-10: Per-package tsconfig overrides look correct and align with repo setupExtending the root config, setting
baseUrl/pathsfor internal imports, scopingrootDirto./src, and addingember-source/typesfor Ember typings all look appropriate and should work cleanly with strict TypeScript in the root config. As per coding guidelines, just ensure ESLint/Prettier are run after these changes.code/frameworks/ember/template/components/button.gjs (1)
1-5: Button template is fine; consider specifyingtype="button"The click wiring via
{{on "click" @onClick}}and@labelrendering look good. To avoid accidental form submission when this button is used inside a<form>, consider addingtype="button"to the<button>element.code/frameworks/ember/template/cli/ts/Header.gts (1)
5-12: Consider makingnameoptional in the user type.The current signature requires
nameto be present whenuserexists. Consider making it optional (name?: string) for more defensive coding, then handle the undefined case in the template (e.g., display a fallback like "Guest").export interface Signature { Args: { - user?: { name: string }; + user?: { name?: string }; onLogin: () => void; onLogout: () => void; onCreateAccount: () => void; }; }Then in the template:
- <span class="welcome">Welcome, <b>{{@user.name}}</b>!</span> + <span class="welcome">Welcome, <b>{{or @user.name "Guest"}}</b>!</span>code/frameworks/ember/template/cli/js/Button.gjs (1)
7-9: Consider validating backgroundColor for security.The modifier directly sets
element.style.backgroundColorfrom args without validation. While this is likely safe in a Storybook context where args are controlled, consider adding validation to prevent potential CSS injection if this component is reused elsewhere.backgroundColor = createModifier((element) => { - element.style.backgroundColor = this.args.backgroundColor ?? ''; + const color = this.args.backgroundColor ?? ''; + // Validate that it's a safe CSS color value + if (color && !/^(#[0-9A-Fa-f]{3,8}|rgba?\(|hsla?\(|[a-z]+)/.test(color)) { + console.warn('Invalid backgroundColor:', color); + return; + } + element.style.backgroundColor = color; });code/frameworks/ember/template/cli/ts/Button.gts (1)
22-24: Consider validating backgroundColor for security.The modifier directly sets
element.style.backgroundColorfrom args without validation. While this is likely safe in a Storybook context where args are controlled, consider adding validation to prevent potential CSS injection if this component is reused elsewhere.backgroundColor = createModifier<{ Element: HTMLElement }>((element) => { - element.style.backgroundColor = this.args.backgroundColor ?? ''; + const color = this.args.backgroundColor ?? ''; + // Validate that it's a safe CSS color value + if (color && !/^(#[0-9A-Fa-f]{3,8}|rgba?\(|hsla?\(|[a-z]+)/.test(color)) { + console.warn('Invalid backgroundColor:', color); + return; + } + element.style.backgroundColor = color; });code/frameworks/ember/template/cli/ts/Button.stories.ts (1)
6-21: Consider adding type annotations for improved developer experience.This TypeScript template could benefit from explicit type annotations using the
MetaandStoryObjtypes from Storybook. This would provide better IntelliSense for developers scaffolding new projects.+import type { Meta, StoryObj } from '@storybook/ember'; import { fn } from 'storybook/test'; import Button from './Button.gts'; // More on how to set up stories at: https://storybook.js.org/docs/writing-stories -export default { +const meta = { title: 'Example/Button', component: Button, // ... rest of config -}; +} satisfies Meta<typeof Button>; + +export default meta; +type Story = StoryObj<typeof meta>;code/frameworks/ember/src/client/render.ts (1)
14-18: Simplify redundant conditional branches.Both branches return the same value. Consider consolidating:
- if (typeof Component === 'function') { - return { Component, args }; - } else if (typeof Component === 'object') { + if (typeof Component === 'function' || typeof Component === 'object') { return { Component, args }; }code/frameworks/ember/src/client/docs/sourceDecorator.ts (1)
27-38: Consider adding a dependency array touseEffect.The
useEffecthook runs on every render since it lacks a dependency array. This could cause unnecessary re-emissions of the source code. Consider adding dependencies if the source should only update when relevant values change:useEffect(() => { const renderedForSource = context?.parameters.docs?.source?.excludeDecorators ? (context.originalStoryFn as StoryFn)(context.args, context) : story; if (!skipSourceRender(context)) { const code = generateGlimmerSource(renderedForSource, context.args, context.argTypes) ?? undefined; emitTransformCode(code, context); source.current = code; } - }); + }, [context.args, context.argTypes]);Verify if running on every render is intentional for this decorator pattern. Other framework source decorators may have similar patterns worth checking for consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (46)
code/frameworks/ember/build-config.ts(1 hunks)code/frameworks/ember/package.json(1 hunks)code/frameworks/ember/src/client/config.ts(1 hunks)code/frameworks/ember/src/client/docs/config.ts(1 hunks)code/frameworks/ember/src/client/docs/sourceDecorator.ts(1 hunks)code/frameworks/ember/src/client/globals.ts(1 hunks)code/frameworks/ember/src/client/index.ts(1 hunks)code/frameworks/ember/src/client/portable-stories.ts(1 hunks)code/frameworks/ember/src/client/preview/config.ts(0 hunks)code/frameworks/ember/src/client/preview/globals.ts(0 hunks)code/frameworks/ember/src/client/preview/jsondoc.ts(0 hunks)code/frameworks/ember/src/client/preview/render.ts(0 hunks)code/frameworks/ember/src/client/preview/types.ts(0 hunks)code/frameworks/ember/src/client/public-types.ts(1 hunks)code/frameworks/ember/src/client/render.ts(1 hunks)code/frameworks/ember/src/client/types.ts(1 hunks)code/frameworks/ember/src/index.ts(1 hunks)code/frameworks/ember/src/node/indexer.ts(1 hunks)code/frameworks/ember/src/preset.ts(2 hunks)code/frameworks/ember/src/server/framework-preset-babel-ember.ts(0 hunks)code/frameworks/ember/src/types.ts(2 hunks)code/frameworks/ember/src/typings.d.ts(0 hunks)code/frameworks/ember/src/util.ts(0 hunks)code/frameworks/ember/template/cli/Button.stories.js(0 hunks)code/frameworks/ember/template/cli/js/Button.gjs(1 hunks)code/frameworks/ember/template/cli/js/Button.stories.js(1 hunks)code/frameworks/ember/template/cli/js/Header.gjs(1 hunks)code/frameworks/ember/template/cli/js/Header.stories.js(1 hunks)code/frameworks/ember/template/cli/js/Page.gjs(1 hunks)code/frameworks/ember/template/cli/js/Page.stories.js(1 hunks)code/frameworks/ember/template/cli/ts/Button.gts(1 hunks)code/frameworks/ember/template/cli/ts/Button.stories.ts(1 hunks)code/frameworks/ember/template/cli/ts/Header.gts(1 hunks)code/frameworks/ember/template/cli/ts/Header.stories.ts(1 hunks)code/frameworks/ember/template/cli/ts/Page.gts(1 hunks)code/frameworks/ember/template/cli/ts/Page.stories.ts(1 hunks)code/frameworks/ember/template/components/button.gjs(1 hunks)code/frameworks/ember/template/components/form.gjs(1 hunks)code/frameworks/ember/template/components/html.gjs(1 hunks)code/frameworks/ember/template/components/index.js(1 hunks)code/frameworks/ember/template/components/pre.gjs(1 hunks)code/frameworks/ember/tsconfig.json(1 hunks)code/lib/cli-storybook/src/sandbox-templates.ts(2 hunks)code/lib/create-storybook/src/generators/EMBER/index.ts(1 hunks)scripts/build/utils/entry-utils.ts(1 hunks)test-storybooks/ember-cli/package.json(0 hunks)
💤 Files with no reviewable changes (10)
- code/frameworks/ember/template/cli/Button.stories.js
- code/frameworks/ember/src/server/framework-preset-babel-ember.ts
- code/frameworks/ember/src/client/preview/jsondoc.ts
- test-storybooks/ember-cli/package.json
- code/frameworks/ember/src/client/preview/config.ts
- code/frameworks/ember/src/typings.d.ts
- code/frameworks/ember/src/client/preview/render.ts
- code/frameworks/ember/src/client/preview/types.ts
- code/frameworks/ember/src/client/preview/globals.ts
- code/frameworks/ember/src/util.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use ESLint and Prettier configurations that are enforced in the codebase
Files:
code/frameworks/ember/src/client/index.tscode/frameworks/ember/src/client/render.tscode/frameworks/ember/template/cli/js/Button.stories.jscode/frameworks/ember/src/node/indexer.tscode/frameworks/ember/template/components/index.jscode/frameworks/ember/package.jsoncode/frameworks/ember/src/client/config.tscode/frameworks/ember/src/client/docs/sourceDecorator.tscode/frameworks/ember/template/cli/js/Header.stories.jscode/frameworks/ember/src/index.tscode/frameworks/ember/tsconfig.jsoncode/frameworks/ember/template/cli/ts/Header.stories.tscode/frameworks/ember/template/cli/ts/Page.stories.tscode/frameworks/ember/src/preset.tscode/frameworks/ember/template/cli/js/Page.stories.jscode/frameworks/ember/src/client/docs/config.tscode/frameworks/ember/src/client/public-types.tscode/frameworks/ember/src/client/portable-stories.tscode/lib/create-storybook/src/generators/EMBER/index.tsscripts/build/utils/entry-utils.tscode/frameworks/ember/template/cli/ts/Button.stories.tscode/frameworks/ember/src/client/globals.tscode/frameworks/ember/src/client/types.tscode/frameworks/ember/build-config.tscode/frameworks/ember/src/types.tscode/lib/cli-storybook/src/sandbox-templates.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Enable TypeScript strict mode
Files:
code/frameworks/ember/src/client/index.tscode/frameworks/ember/src/client/render.tscode/frameworks/ember/src/node/indexer.tscode/frameworks/ember/src/client/config.tscode/frameworks/ember/src/client/docs/sourceDecorator.tscode/frameworks/ember/src/index.tscode/frameworks/ember/template/cli/ts/Header.stories.tscode/frameworks/ember/template/cli/ts/Page.stories.tscode/frameworks/ember/src/preset.tscode/frameworks/ember/src/client/docs/config.tscode/frameworks/ember/src/client/public-types.tscode/frameworks/ember/src/client/portable-stories.tscode/lib/create-storybook/src/generators/EMBER/index.tsscripts/build/utils/entry-utils.tscode/frameworks/ember/template/cli/ts/Button.stories.tscode/frameworks/ember/src/client/globals.tscode/frameworks/ember/src/client/types.tscode/frameworks/ember/build-config.tscode/frameworks/ember/src/types.tscode/lib/cli-storybook/src/sandbox-templates.ts
code/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
code/**/*.{ts,tsx,js,jsx,mjs}: Use server-side logger from 'storybook/internal/node-logger' for Node.js code
Use client-side logger from 'storybook/internal/client-logger' for browser code
Do not use console.log, console.warn, or console.error directly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/frameworks/ember/src/client/index.tscode/frameworks/ember/src/client/render.tscode/frameworks/ember/template/cli/js/Button.stories.jscode/frameworks/ember/src/node/indexer.tscode/frameworks/ember/template/components/index.jscode/frameworks/ember/src/client/config.tscode/frameworks/ember/src/client/docs/sourceDecorator.tscode/frameworks/ember/template/cli/js/Header.stories.jscode/frameworks/ember/src/index.tscode/frameworks/ember/template/cli/ts/Header.stories.tscode/frameworks/ember/template/cli/ts/Page.stories.tscode/frameworks/ember/src/preset.tscode/frameworks/ember/template/cli/js/Page.stories.jscode/frameworks/ember/src/client/docs/config.tscode/frameworks/ember/src/client/public-types.tscode/frameworks/ember/src/client/portable-stories.tscode/lib/create-storybook/src/generators/EMBER/index.tscode/frameworks/ember/template/cli/ts/Button.stories.tscode/frameworks/ember/src/client/globals.tscode/frameworks/ember/src/client/types.tscode/frameworks/ember/build-config.tscode/frameworks/ember/src/types.tscode/lib/cli-storybook/src/sandbox-templates.ts
code/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Export functions that need to be tested from their modules
Files:
code/frameworks/ember/src/client/index.tscode/frameworks/ember/src/client/render.tscode/frameworks/ember/template/cli/js/Button.stories.jscode/frameworks/ember/src/node/indexer.tscode/frameworks/ember/template/components/index.jscode/frameworks/ember/src/client/config.tscode/frameworks/ember/src/client/docs/sourceDecorator.tscode/frameworks/ember/template/cli/js/Header.stories.jscode/frameworks/ember/src/index.tscode/frameworks/ember/template/cli/ts/Header.stories.tscode/frameworks/ember/template/cli/ts/Page.stories.tscode/frameworks/ember/src/preset.tscode/frameworks/ember/template/cli/js/Page.stories.jscode/frameworks/ember/src/client/docs/config.tscode/frameworks/ember/src/client/public-types.tscode/frameworks/ember/src/client/portable-stories.tscode/lib/create-storybook/src/generators/EMBER/index.tscode/frameworks/ember/template/cli/ts/Button.stories.tscode/frameworks/ember/src/client/globals.tscode/frameworks/ember/src/client/types.tscode/frameworks/ember/build-config.tscode/frameworks/ember/src/types.tscode/lib/cli-storybook/src/sandbox-templates.ts
code/**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
code/**/*.{js,jsx,json,html,ts,tsx,mjs}: Run Prettier with --write flag to format code before committing
Run ESLint with yarn lint:js:cmd to check for linting issues and fix errors before committing
Files:
code/frameworks/ember/src/client/index.tscode/frameworks/ember/src/client/render.tscode/frameworks/ember/template/cli/js/Button.stories.jscode/frameworks/ember/src/node/indexer.tscode/frameworks/ember/template/components/index.jscode/frameworks/ember/package.jsoncode/frameworks/ember/src/client/config.tscode/frameworks/ember/src/client/docs/sourceDecorator.tscode/frameworks/ember/template/cli/js/Header.stories.jscode/frameworks/ember/src/index.tscode/frameworks/ember/tsconfig.jsoncode/frameworks/ember/template/cli/ts/Header.stories.tscode/frameworks/ember/template/cli/ts/Page.stories.tscode/frameworks/ember/src/preset.tscode/frameworks/ember/template/cli/js/Page.stories.jscode/frameworks/ember/src/client/docs/config.tscode/frameworks/ember/src/client/public-types.tscode/frameworks/ember/src/client/portable-stories.tscode/lib/create-storybook/src/generators/EMBER/index.tscode/frameworks/ember/template/cli/ts/Button.stories.tscode/frameworks/ember/src/client/globals.tscode/frameworks/ember/src/client/types.tscode/frameworks/ember/build-config.tscode/frameworks/ember/src/types.tscode/lib/cli-storybook/src/sandbox-templates.ts
🧠 Learnings (22)
📓 Common learnings
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.
📚 Learning: 2025-11-28T14:50:24.889Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Applies to code/**/*.{ts,tsx,js,jsx,mjs} : Use client-side logger from 'storybook/internal/client-logger' for browser code
Applied to files:
code/frameworks/ember/src/client/index.tscode/frameworks/ember/package.jsoncode/frameworks/ember/src/client/config.tscode/frameworks/ember/src/index.tscode/frameworks/ember/tsconfig.jsoncode/frameworks/ember/src/client/globals.tscode/frameworks/ember/src/client/types.ts
📚 Learning: 2025-11-28T14:50:24.889Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Applies to code/**/*.{ts,tsx,js,jsx} : Export functions that need to be tested from their modules
Applied to files:
code/frameworks/ember/src/client/index.tscode/frameworks/ember/src/index.tscode/frameworks/ember/build-config.ts
📚 Learning: 2025-09-18T20:51:06.618Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/viewport/components/Tool.tsx:38-39
Timestamp: 2025-09-18T20:51:06.618Z
Learning: The useGlobals hook from storybook/manager-api returns a tuple where the third element (storyGlobals) is typed as Globals, not Globals | undefined. This means TypeScript guarantees it's always defined, making the `in` operator safe to use without additional null checks.
Applied to files:
code/frameworks/ember/src/client/index.tscode/frameworks/ember/template/components/index.jscode/frameworks/ember/src/client/globals.tscode/frameworks/ember/src/types.ts
📚 Learning: 2025-09-18T20:51:06.618Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/viewport/components/Tool.tsx:38-39
Timestamp: 2025-09-18T20:51:06.618Z
Learning: In viewport tool code, when using the `useGlobals` hook from storybook/manager-api, the third returned value `storyGlobals` is guaranteed by TypeScript to be defined (not undefined/null), making the `in` operator safe to use without additional null checks.
Applied to files:
code/frameworks/ember/src/client/index.tscode/frameworks/ember/template/components/index.jscode/frameworks/ember/src/client/globals.ts
📚 Learning: 2025-11-28T14:50:24.889Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Follow existing patterns and conventions in the Storybook codebase
Applied to files:
code/frameworks/ember/template/cli/ts/Page.gtscode/frameworks/ember/template/cli/js/Button.stories.jscode/frameworks/ember/template/components/index.jscode/frameworks/ember/template/cli/js/Page.gjscode/frameworks/ember/template/cli/ts/Header.stories.tscode/frameworks/ember/template/cli/ts/Page.stories.tscode/frameworks/ember/template/cli/js/Page.stories.jscode/frameworks/ember/template/cli/ts/Button.stories.tscode/frameworks/ember/src/client/globals.tscode/lib/cli-storybook/src/sandbox-templates.ts
📚 Learning: 2025-11-05T09:38:47.712Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.
Applied to files:
code/frameworks/ember/src/client/render.tscode/frameworks/ember/src/client/docs/sourceDecorator.tsscripts/build/utils/entry-utils.tscode/frameworks/ember/src/types.tscode/lib/cli-storybook/src/sandbox-templates.ts
📚 Learning: 2025-11-05T09:37:25.920Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/tooltip/WithTooltip.tsx:54-96
Timestamp: 2025-11-05T09:37:25.920Z
Learning: Repo: storybookjs/storybook — In code/core/src/components/components/tooltip/WithTooltip.tsx, the legacy WithTooltip implementation is intentionally reintroduced for backward compatibility and is deprecated; maintainers (per Sidnioulz) do not want maintenance or improvements on it. Prefer WithTooltipNew/Popover; avoid suggesting changes to WithTooltip.* going forward.
Applied to files:
code/frameworks/ember/src/client/render.tscode/frameworks/ember/template/components/index.jscode/frameworks/ember/src/client/docs/sourceDecorator.tscode/frameworks/ember/src/types.ts
📚 Learning: 2025-11-05T09:36:55.944Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Tabs/Tabs.stories.tsx:222-227
Timestamp: 2025-11-05T09:36:55.944Z
Learning: Repo: storybookjs/storybook PR: 32458 — In code/core/src/components/components/Button/Button.tsx (React/TypeScript), ButtonProps includes ariaLabel?: string | false and the component maps it to the DOM aria-label. Convention: ariaLabel is mandatory on all Button usages — provide a descriptive string for icon-only buttons; set ariaLabel=false when the button’s children already serve as the accessible name. Do not suggest using a raw aria-label prop on Button call sites.
Applied to files:
code/frameworks/ember/template/cli/js/Button.stories.jscode/frameworks/ember/template/cli/js/Button.gjscode/frameworks/ember/template/components/index.jscode/frameworks/ember/template/components/button.gjscode/frameworks/ember/template/cli/ts/Button.gtscode/frameworks/ember/template/cli/ts/Button.stories.ts
📚 Learning: 2025-09-24T09:39:39.233Z
Learnt from: ndelangen
Repo: storybookjs/storybook PR: 32507
File: code/core/src/manager/globals/globals-module-info.ts:25-33
Timestamp: 2025-09-24T09:39:39.233Z
Learning: In Storybook, storybook/actions/decorator is a preview-only entrypoint and should not be included in manager globals configuration. The duplicatedKeys array in code/core/src/manager/globals/globals-module-info.ts is specifically for manager-side externalization, not preview entrypoints.
Applied to files:
code/frameworks/ember/template/components/index.jscode/frameworks/ember/src/client/docs/sourceDecorator.tscode/frameworks/ember/src/preset.tscode/frameworks/ember/src/client/docs/config.tscode/frameworks/ember/src/client/public-types.tscode/lib/create-storybook/src/generators/EMBER/index.tscode/frameworks/ember/src/client/globals.tscode/frameworks/ember/src/types.ts
📚 Learning: 2025-11-28T14:50:24.889Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Applies to **/*.{js,jsx,json,html,ts,tsx,mjs} : Use ESLint and Prettier configurations that are enforced in the codebase
Applied to files:
code/frameworks/ember/package.jsoncode/frameworks/ember/tsconfig.jsoncode/frameworks/ember/build-config.tscode/lib/cli-storybook/src/sandbox-templates.ts
📚 Learning: 2025-11-24T17:49:31.838Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-24T17:49:31.838Z
Learning: Applies to code/vitest.workspace.ts : Vitest configuration is centralized in `code/vitest.workspace.ts` for workspace setup
Applied to files:
code/frameworks/ember/package.jsoncode/frameworks/ember/src/types.ts
📚 Learning: 2025-09-17T07:31:04.432Z
Learnt from: ndelangen
Repo: storybookjs/storybook PR: 32484
File: code/core/package.json:326-326
Timestamp: 2025-09-17T07:31:04.432Z
Learning: In Storybook's core package, dependencies like `open` are bundled into the final distribution during the build process, so they should remain in devDependencies rather than being moved to dependencies. End users don't need these packages as separate runtime dependencies since they're included in the bundled code.
Applied to files:
code/frameworks/ember/package.jsonscripts/build/utils/entry-utils.ts
📚 Learning: 2025-09-29T13:20:23.346Z
Learnt from: mrginglymus
Repo: storybookjs/storybook PR: 32556
File: code/core/package.json:309-313
Timestamp: 2025-09-29T13:20:23.346Z
Learning: The `fast-printf` dependency in Storybook's core package is bundled into the final distribution during the build process, so it should remain in devDependencies rather than being moved to dependencies, following the same pattern as other bundled dependencies like `open`.
Applied to files:
code/frameworks/ember/package.jsonscripts/build/utils/entry-utils.ts
📚 Learning: 2025-11-28T14:50:24.889Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Applies to code/**/*.{ts,tsx,js,jsx,mjs} : Use server-side logger from 'storybook/internal/node-logger' for Node.js code
Applied to files:
code/frameworks/ember/package.jsoncode/frameworks/ember/tsconfig.jsoncode/frameworks/ember/src/client/globals.ts
📚 Learning: 2025-09-24T13:04:58.631Z
Learnt from: cylewaitforit
Repo: storybookjs/storybook PR: 31965
File: code/lib/eslint-plugin/src/rules/only-csf3.ts:31-33
Timestamp: 2025-09-24T13:04:58.631Z
Learning: The Storybook ESLint plugin supports ESLint v8.57+ where context.sourceCode is already available as a property since it was introduced in v8.40.0, so no fallback to context.getSourceCode() is needed in rules.
Applied to files:
code/frameworks/ember/src/client/docs/sourceDecorator.ts
📚 Learning: 2025-11-28T14:50:24.889Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Applies to **/*.{ts,tsx} : Enable TypeScript strict mode
Applied to files:
code/frameworks/ember/tsconfig.jsoncode/frameworks/ember/build-config.tscode/lib/cli-storybook/src/sandbox-templates.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Document complex mock behaviors in Vitest tests
Applied to files:
code/frameworks/ember/template/cli/js/Page.stories.jscode/frameworks/ember/src/types.tscode/lib/cli-storybook/src/sandbox-templates.ts
📚 Learning: 2025-10-01T15:24:01.060Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32594
File: code/core/src/components/components/Popover/WithPopover.tsx:7-9
Timestamp: 2025-10-01T15:24:01.060Z
Learning: In the Storybook repository, "react-aria-components/patched-dist/*" (e.g., "react-aria-components/patched-dist/Dialog", "react-aria-components/patched-dist/Popover", "react-aria-components/patched-dist/Tooltip") are valid import paths created by a patch applied to the react-aria-components package. These imports should not be flagged as broken or invalid until a maintainer explicitly states they are no longer acceptable.
Applied to files:
scripts/build/utils/entry-utils.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mocking only a subset of required dependencies in Vitest tests
Applied to files:
code/lib/cli-storybook/src/sandbox-templates.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mocking without the `spy: true` option in Vitest tests
Applied to files:
code/lib/cli-storybook/src/sandbox-templates.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Implement mock behaviors in `beforeEach` blocks in Vitest tests
Applied to files:
code/lib/cli-storybook/src/sandbox-templates.ts
🧬 Code graph analysis (10)
code/frameworks/ember/template/cli/js/Button.stories.js (1)
code/frameworks/ember/template/cli/ts/Button.stories.ts (2)
Primary(23-29)Secondary(31-37)
code/frameworks/ember/src/client/config.ts (3)
code/frameworks/ember/src/client/docs/config.ts (1)
parameters(6-13)code/frameworks/ember/src/client/public-types.ts (1)
Parameters(15-15)code/frameworks/ember/src/client/types.ts (1)
Parameters(22-25)
code/frameworks/ember/src/client/docs/sourceDecorator.ts (2)
code/frameworks/ember/src/client/public-types.ts (4)
Parameters(15-15)EmberRenderer(16-16)StoryFn(30-30)Args(15-15)code/frameworks/ember/src/client/types.ts (2)
Parameters(22-25)EmberRenderer(12-20)
code/frameworks/ember/template/cli/js/Header.stories.js (1)
code/frameworks/ember/template/cli/ts/Header.stories.ts (2)
LoggedIn(24-30)LoggedOut(32-32)
code/frameworks/ember/template/cli/ts/Header.stories.ts (2)
code/frameworks/ember/template/cli/js/Header.stories.js (5)
meta(5-17)LoggedIn(21-27)LoggedIn(21-27)LoggedOut(29-29)LoggedOut(29-29)code/frameworks/ember/src/client/public-types.ts (2)
Meta(23-23)StoryObj(37-37)
code/frameworks/ember/src/preset.ts (4)
code/core/src/types/modules/core-common.ts (1)
PresetProperty(657-659)code/builders/builder-vite/src/index.ts (1)
withoutVitePlugins(14-14)code/frameworks/ember/src/types.ts (1)
StorybookConfig(33-38)code/frameworks/ember/src/node/indexer.ts (1)
emberIndexer(19-25)
code/frameworks/ember/src/client/docs/config.ts (4)
code/frameworks/ember/src/client/config.ts (1)
parameters(6-11)code/frameworks/ember/src/client/public-types.ts (1)
Parameters(15-15)code/frameworks/ember/src/client/types.ts (1)
Parameters(22-25)code/frameworks/ember/src/client/docs/sourceDecorator.ts (1)
sourceDecorator(23-41)
code/frameworks/ember/src/client/public-types.ts (1)
code/frameworks/ember/src/client/types.ts (2)
EmberRenderer(12-20)StoryContext(27-29)
code/frameworks/ember/src/client/portable-stories.ts (3)
code/core/src/preview-api/modules/preview-web/docs-context/DocsContext.ts (1)
projectAnnotations(109-115)code/frameworks/ember/src/client/public-types.ts (1)
EmberRenderer(16-16)code/frameworks/ember/src/client/types.ts (1)
EmberRenderer(12-20)
code/frameworks/ember/src/client/types.ts (1)
code/frameworks/ember/src/client/public-types.ts (3)
EmberRenderer(16-16)Parameters(15-15)StoryContext(41-41)
🪛 Biome (2.1.2)
code/frameworks/ember/template/components/index.js
[error] 1-1: Do not shadow the global "globalThis" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: normal
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (32)
scripts/build/utils/entry-utils.ts (1)
64-66: LGTM!The explicit inclusion of these Ember packages is correctly documented, and the comment helpfully explains that most
@emberpackages are virtual and need to be excluded.code/frameworks/ember/template/components/pre.gjs (1)
1-5: LGTM!The
formatContenthelper is simple and correctly handles both text and object rendering.code/lib/create-storybook/src/generators/EMBER/index.ts (1)
14-14: LGTM!Adding
.gtsand.gjsextensions correctly supports Ember's template tag format, which is essential for the new Vite-based integration.code/frameworks/ember/src/node/indexer.ts (1)
8-24: LGTM!The indexer correctly uses the content-tag Preprocessor to parse Ember template files before CSF processing. The test pattern
/\.stories\.g[tj]s$/appropriately matches both.gjsand.gtsstory files.code/frameworks/ember/package.json (2)
32-52: LGTM!The new export structure correctly exposes the client-side entry points and updates the preset export to the modern object format with explicit types and default fields.
64-79: LGTM!The dependency updates correctly reflect the Vite migration:
@storybook/builder-vitereplaces the webpack buildercontent-tagenables parsing of.gjs/.gtsfiles- Peer dependency on
ember-source >=6.8.0aligns with the PR objective (Ember 6.8 ships Vite as default)- Vite peer dependency range supports multiple major versions
code/frameworks/ember/template/components/index.js (1)
8-9: LGTM!The global component registry setup correctly exposes the Ember template components for Storybook to use during rendering.
code/frameworks/ember/template/components/html.gjs (1)
1-1: Verify XSS safety for unescaped HTML rendering.The triple-brace syntax
{{{@content}}}renders unescaped HTML, which can be an XSS vector if@contentincludes user-supplied data. Ensure that:
- This component is only used with trusted, developer-controlled content (not user-supplied data)
- Add documentation clarifying the security implications and appropriate use cases
- Consider whether a sanitization wrapper or safe-by-default alternative should be provided
code/frameworks/ember/src/client/globals.ts (1)
1-1: Ember STORYBOOK_ENV global is set in the expected waySetting
globalThis.STORYBOOK_ENV = 'ember'as a side-effect module matches the established pattern for framework env flags in Storybook and should integrate cleanly with the rest of the tooling.code/frameworks/ember/template/cli/js/Button.stories.js (1)
1-37: Button JS stories mirror the TS template and are wired correctlyImport paths, argTypes (including
onClickaction) and defaultargswithfn()match Storybook’s recommended patterns and are consistent with the TS Button stories variant.code/frameworks/ember/src/index.ts (1)
1-2: Index barrel now correctly exposes both core types and client surfaceRe-exporting from
./typesand./client/indexgives consumers a unified Ember entrypoint. Just double-check there are no overlapping export names between those modules that could cause ESM re-export conflicts.code/frameworks/ember/src/client/portable-stories.ts (1)
1-42: EmbersetProjectAnnotationswrapper cleanly delegates while registering defaultsThe function correctly installs the internal Ember render project annotations via
setDefaultProjectAnnotationsbefore delegating to the originalsetProjectAnnotations, and the return type narrowing toNormalizedProjectAnnotations<EmberRenderer>aligns with the renderer-specific typings for downstream consumers.code/frameworks/ember/src/client/docs/config.ts (1)
1-15: Docs config correctly wires dynamic HTML source and Ember source decoratorThe exported
parametersobject focuses docs-only settings (docs.sourcewithSourceType.DYNAMIC,language: 'html'), and thedecoratorsarray registeringsourceDecoratormatches the intended Ember docs/source pipeline.code/frameworks/ember/src/client/config.ts (1)
1-13: LGTM!Clean configuration module that properly exports rendering utilities and sets up Ember-specific Storybook parameters.
code/frameworks/ember/template/cli/js/Page.stories.js (1)
1-30: LGTM!Well-structured interaction test that follows Storybook best practices for component testing. The play function properly tests the login flow with appropriate assertions.
code/frameworks/ember/template/cli/ts/Header.stories.ts (1)
1-32: LGTM!Type-safe story definitions that properly leverage the component's Signature type and follow Storybook CSF3 conventions.
code/frameworks/ember/src/client/index.ts (1)
1-5: LGTM!Clean barrel export that establishes the public API surface for the Ember client package.
code/frameworks/ember/template/cli/ts/Page.stories.ts (1)
1-32: LGTM!Type-safe story definitions with comprehensive interaction testing that validates the login/logout flow.
code/frameworks/ember/template/cli/js/Header.stories.js (1)
1-29: LGTM!The story file follows Storybook CSF conventions correctly. The meta configuration with action spies using
fn(), layout parameter, and story variants (LoggedIn/LoggedOut) are well-structured and consistent with the TypeScript variant.code/frameworks/ember/template/cli/js/Page.gjs (1)
6-94: LGTM on component structure and template.The Glimmer component is well-structured with proper use of
@trackedfor reactivity and arrow function handlers to preservethiscontext. The template correctly passes user state and action handlers to the Header component.code/frameworks/ember/src/client/render.ts (1)
84-91: LGTM onupdateArgsimplementation.The function correctly handles reactive updates by first removing stale keys before assigning new values, which ensures proper tracking invalidation in Ember's reactivity system.
code/frameworks/ember/src/client/types.ts (2)
12-29: LGTM on type definitions.The
EmberRendererinterface correctly extendsWebRendererwith Ember-specific types. The comment explaining theobjecttyping forcomponentis helpful, and theStoryContexttype properly composes default context with Ember parameters.
7-10: No action needed.ShowErrorArgsis a standardized exported interface used consistently across all Storybook renderers and frameworks (preact, server, vue3, react, web-components, html, and ember). This is intentional API design for error handling.code/frameworks/ember/template/cli/ts/Page.gts (2)
1-20: LGTM! Clean Glimmer component implementation.The component correctly uses
@trackedfor reactive state and arrow functions for event handlers to preservethisbinding. The user state management is straightforward.One observation:
onLoginandonCreateAccounthave identical implementations (lines 9-11 and 17-19). This is likely intentional for template purposes, but worth confirming if different behavior is expected in a real application.
21-94: Template implementation looks good.The Header component integration with args (
@user,@onLogin,@onLogout,@onCreateAccount) follows Glimmer conventions. External links correctly includerel="noopener noreferrer"for security.code/frameworks/ember/src/client/docs/sourceDecorator.ts (1)
9-21: Source rendering logic is well-structured.The
skipSourceRenderfunction correctly handles the various source configuration scenarios:
- Forces rendering when
SourceType.DYNAMICis set- Skips rendering appropriately when code is provided or for non-args stories
code/frameworks/ember/src/client/public-types.ts (1)
1-42: Well-structured public type definitions.The type aliases are correctly bound to
EmberRendererand follow the established patterns from other framework integrations. The JSDoc comments with documentation links enhance developer experience.code/frameworks/ember/src/preset.ts (2)
12-29: Preview annotations setup is correct.The conditional inclusion of docs configuration based on
docsEnabledand the use ofimport.meta.resolvewithfileURLToPathfor ESM compatibility is well-implemented.
54-56: Indexer registration looks good.Prepending
emberIndexerensures Ember.stories.gtsand.stories.gjsfiles are indexed before other indexers process them.code/frameworks/ember/src/types.ts (1)
1-38: Type definitions correctly updated for Vite migration.The changes properly align the framework and builder types with the Vite-based architecture:
FrameworkNameupdated to@storybook/emberBuilderNameupdated to@storybook/builder-viteStorybookConfignow composes withStorybookConfigViteThe use of
CompatibleString<T>maintains backward compatibility while enforcing the expected literal types.code/frameworks/ember/build-config.ts (1)
10-21: LGTM!The new browser export entries for client modules are well-structured and consistent with the existing configuration format. These changes appropriately support the Vite-based Ember integration.
code/lib/cli-storybook/src/sandbox-templates.ts (1)
1003-1003: Verify inclusion ofember/default-tsin CI despiteinDevelopmentflag.The
ember/default-tstemplate is marked withinDevelopment: true(line 760), which per line 79 should "avoid using it in CI." However, it's now included in thenormalarray that runs in CI. While the template has extensiveskipTasksthat may make this safe, this appears to contradict the documented guideline.Compare with
qwik-vite/default-ts(line 735), which hasinDevelopment: trueand is NOT included in thenormalarray.Is this inclusion intentional to provide CI coverage during development, or should
ember/default-tsbe excluded from thenormalarray until theinDevelopmentflag is removed?
| export default class Form extends Component { | ||
| @tracked complete = false; | ||
| @tracked value = ''; | ||
|
|
||
| onChange = (event) => { | ||
| this.value = event.currentTarget.value; | ||
| }; | ||
|
|
||
| onSubmit = (event) => { | ||
| event.preventDefault(); | ||
| this.args.onSuccess(this.value); | ||
|
|
||
| setTimeout(() => { this.complete = true; }, 500); | ||
| setTimeout(() => { this.complete = false; }, 1500); | ||
| }; |
There was a problem hiding this comment.
Clean up timeouts on component destruction.
The setTimeout calls on lines 17-18 will continue to execute even if the component is unmounted, potentially attempting to update tracked properties on a destroyed component. This can lead to memory leaks and errors.
Consider using Ember's @ember/destroyable to register cleanup:
+import { registerDestructor } from '@ember/destroyable';
+
export default class Form extends Component {
@tracked complete = false;
@tracked value = '';
+
+ timeouts = [];
+
+ constructor() {
+ super(...arguments);
+ registerDestructor(this, () => {
+ this.timeouts.forEach(clearTimeout);
+ });
+ }
onChange = (event) => {
this.value = event.currentTarget.value;
};
onSubmit = (event) => {
event.preventDefault();
this.args.onSuccess(this.value);
- setTimeout(() => { this.complete = true; }, 500);
- setTimeout(() => { this.complete = false; }, 1500);
+ this.timeouts.push(setTimeout(() => { this.complete = true; }, 500));
+ this.timeouts.push(setTimeout(() => { this.complete = false; }, 1500));
};🤖 Prompt for AI Agents
In code/frameworks/ember/template/components/form.gjs around lines 5-19, the two
setTimeout calls aren't cleaned up on component teardown; store their timer IDs
on the instance (e.g. this._completeTimer / this._resetTimer), import
registerDestructor from '@ember/destroyable' and register a destructor that
clears any non-null timers with clearTimeout to avoid updating tracked props
after unmount and prevent leaks; ensure timers are nulled after clearing and
consider clearing existing timers before setting new ones.
| @@ -1,0 +1,9 @@ | |||
| import { global as globalThis } from '@storybook/global'; | |||
There was a problem hiding this comment.
Avoid shadowing the global globalThis identifier.
Renaming global to globalThis creates confusion because it shadows the native globalThis available in modern JavaScript environments.
Consider one of these alternatives:
Option 1 (recommended): Use native globalThis directly if the environment supports it:
-import { global as globalThis } from '@storybook/global';
+const globalThis = typeof window !== 'undefined' ? window : global;Option 2: Keep the import but use a different name:
-import { global as globalThis } from '@storybook/global';
+import { global } from '@storybook/global';
-globalThis.__TEMPLATE_COMPONENTS__ = { Button, Pre, Form, Html };
-globalThis.storybookRenderer = 'ember';
+global.__TEMPLATE_COMPONENTS__ = { Button, Pre, Form, Html };
+global.storybookRenderer = 'ember';Based on learnings, this follows the pattern used elsewhere in Storybook for cross-environment global access.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { global as globalThis } from '@storybook/global'; | |
| const globalThis = typeof window !== 'undefined' ? window : global; |
🧰 Tools
🪛 Biome (2.1.2)
[error] 1-1: Do not shadow the global "globalThis" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
🤖 Prompt for AI Agents
In code/frameworks/ember/template/components/index.js around line 1, the import
renames Storybook's exported `global` to `globalThis`, shadowing the native
`globalThis`. Replace this by either using the native globalThis directly (if
target environments support it) or keep the imported name but rename it to a
non-conflicting identifier (e.g., `storybookGlobal`) and update all usages
accordingly so the native `globalThis` is not shadowed.
Fix integration issues to get Ember rendering.
256065b to
02c8df6
Compare
There was a problem hiding this comment.
Actionable comments posted: 10
♻️ Duplicate comments (2)
code/frameworks/ember/template/components/index.js (1)
1-9: Fix Biome lint error: don’t shadow the nativeglobalThis.
This still renames@storybook/global’sglobaltoglobalThis(Line 1), which Biome flags and is easy to misread at runtime.-import { global as globalThis } from '@storybook/global'; +import { global as storybookGlobal } from '@storybook/global'; import Button from './button.gjs'; import Form from './form.gjs'; import Html from './html.gjs'; import Pre from './pre.gjs'; -globalThis.__TEMPLATE_COMPONENTS__ = { Button, Pre, Form, Html }; -globalThis.storybookRenderer = 'ember'; +storybookGlobal.__TEMPLATE_COMPONENTS__ = { Button, Pre, Form, Html }; +storybookGlobal.storybookRenderer = 'ember';code/frameworks/ember/src/client/docs/sourceDecorator.ts (1)
65-86: Consider supporting objects/arrays via Glimmerhash/arrayhelpers (currently dropped).
Right now non-primitive args are omitted from the generated source, which will surprise users once they pass structured args. (This is the same underlying concern as the earlier “JSON.stringify is invalid Glimmer” feedback, but the current behavior now “drops” instead of “renders invalid”.)
🧹 Nitpick comments (2)
code/frameworks/ember/src/client/config.ts (1)
1-13: Preferstorybook/internalParameters and Ember Parameters via intersection for stronger guarantees.
This keeps Storybook compatibility but still enforcesrenderer: 'ember'at compile time.import { enhanceArgTypes } from 'storybook/internal/docs-tools'; -import type { ArgTypesEnhancer, Parameters } from 'storybook/internal/types'; +import type { ArgTypesEnhancer, Parameters as SBParameters } from 'storybook/internal/types'; +import type { Parameters as EmberParameters } from './types'; export { renderToCanvas, render } from './render'; -export const parameters: Parameters = { +export const parameters: SBParameters & EmberParameters = { renderer: 'ember', docs: { story: { inline: true }, }, };code/frameworks/ember/src/client/docs/config.ts (1)
1-15: Type decorators asDecoratorFunction<EmberRenderer>[]to keep Ember context typed.import { SourceType } from 'storybook/internal/docs-tools'; import type { DecoratorFunction, Parameters } from 'storybook/internal/types'; +import type { EmberRenderer } from '../types'; import { sourceDecorator } from './sourceDecorator'; export const parameters: Parameters = { docs: { source: { type: SourceType.DYNAMIC, language: 'html', }, }, }; -export const decorators: DecoratorFunction[] = [sourceDecorator]; +export const decorators: DecoratorFunction<EmberRenderer>[] = [sourceDecorator];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (39)
code/frameworks/ember/build-config.ts(1 hunks)code/frameworks/ember/package.json(1 hunks)code/frameworks/ember/src/client/config.ts(1 hunks)code/frameworks/ember/src/client/docs/config.ts(1 hunks)code/frameworks/ember/src/client/docs/sourceDecorator.ts(1 hunks)code/frameworks/ember/src/client/globals.ts(1 hunks)code/frameworks/ember/src/client/index.ts(1 hunks)code/frameworks/ember/src/client/portable-stories.ts(1 hunks)code/frameworks/ember/src/client/preview/config.ts(0 hunks)code/frameworks/ember/src/client/preview/globals.ts(0 hunks)code/frameworks/ember/src/client/preview/jsondoc.ts(0 hunks)code/frameworks/ember/src/client/preview/render.ts(0 hunks)code/frameworks/ember/src/client/preview/types.ts(0 hunks)code/frameworks/ember/src/client/public-types.ts(1 hunks)code/frameworks/ember/src/client/render.ts(1 hunks)code/frameworks/ember/src/client/types.ts(1 hunks)code/frameworks/ember/src/node/indexer.ts(1 hunks)code/frameworks/ember/src/preset.ts(2 hunks)code/frameworks/ember/template/cli/js/Button.gjs(1 hunks)code/frameworks/ember/template/cli/js/Button.stories.js(1 hunks)code/frameworks/ember/template/cli/js/Header.gjs(1 hunks)code/frameworks/ember/template/cli/js/Header.stories.js(1 hunks)code/frameworks/ember/template/cli/js/Page.gjs(1 hunks)code/frameworks/ember/template/cli/js/Page.stories.js(1 hunks)code/frameworks/ember/template/cli/ts/Button.gts(1 hunks)code/frameworks/ember/template/cli/ts/Button.stories.ts(1 hunks)code/frameworks/ember/template/cli/ts/Header.gts(1 hunks)code/frameworks/ember/template/cli/ts/Header.stories.ts(1 hunks)code/frameworks/ember/template/cli/ts/Page.gts(1 hunks)code/frameworks/ember/template/cli/ts/Page.stories.ts(1 hunks)code/frameworks/ember/template/components/button.gjs(1 hunks)code/frameworks/ember/template/components/form.gjs(1 hunks)code/frameworks/ember/template/components/html.gjs(1 hunks)code/frameworks/ember/template/components/index.js(1 hunks)code/frameworks/ember/template/components/pre.gjs(1 hunks)code/frameworks/ember/tsconfig.json(1 hunks)code/lib/cli-storybook/src/sandbox-templates.ts(2 hunks)code/lib/create-storybook/src/generators/EMBER/index.ts(1 hunks)scripts/build/utils/entry-utils.ts(1 hunks)
💤 Files with no reviewable changes (5)
- code/frameworks/ember/src/client/preview/jsondoc.ts
- code/frameworks/ember/src/client/preview/config.ts
- code/frameworks/ember/src/client/preview/globals.ts
- code/frameworks/ember/src/client/preview/render.ts
- code/frameworks/ember/src/client/preview/types.ts
🚧 Files skipped from review as they are similar to previous changes (19)
- code/frameworks/ember/package.json
- code/frameworks/ember/template/cli/js/Header.stories.js
- code/frameworks/ember/template/cli/js/Button.stories.js
- code/frameworks/ember/src/client/globals.ts
- code/frameworks/ember/template/cli/ts/Button.stories.ts
- code/frameworks/ember/template/cli/js/Page.stories.js
- code/frameworks/ember/template/cli/ts/Header.gts
- code/frameworks/ember/template/cli/js/Header.gjs
- code/frameworks/ember/src/client/index.ts
- code/frameworks/ember/template/components/pre.gjs
- code/frameworks/ember/template/cli/js/Page.gjs
- code/frameworks/ember/template/cli/ts/Page.gts
- code/frameworks/ember/template/cli/ts/Page.stories.ts
- code/frameworks/ember/template/cli/ts/Button.gts
- code/frameworks/ember/src/node/indexer.ts
- code/frameworks/ember/template/components/form.gjs
- scripts/build/utils/entry-utils.ts
- code/frameworks/ember/template/components/html.gjs
- code/frameworks/ember/template/cli/js/Button.gjs
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use ESLint and Prettier configurations that are enforced in the codebase
Files:
code/frameworks/ember/src/client/portable-stories.tscode/frameworks/ember/src/client/types.tscode/frameworks/ember/tsconfig.jsoncode/lib/create-storybook/src/generators/EMBER/index.tscode/frameworks/ember/src/preset.tscode/frameworks/ember/src/client/render.tscode/frameworks/ember/src/client/config.tscode/frameworks/ember/src/client/public-types.tscode/frameworks/ember/src/client/docs/sourceDecorator.tscode/frameworks/ember/template/cli/ts/Header.stories.tscode/lib/cli-storybook/src/sandbox-templates.tscode/frameworks/ember/src/client/docs/config.tscode/frameworks/ember/build-config.tscode/frameworks/ember/template/components/index.js
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Enable TypeScript strict mode
Files:
code/frameworks/ember/src/client/portable-stories.tscode/frameworks/ember/src/client/types.tscode/lib/create-storybook/src/generators/EMBER/index.tscode/frameworks/ember/src/preset.tscode/frameworks/ember/src/client/render.tscode/frameworks/ember/src/client/config.tscode/frameworks/ember/src/client/public-types.tscode/frameworks/ember/src/client/docs/sourceDecorator.tscode/frameworks/ember/template/cli/ts/Header.stories.tscode/lib/cli-storybook/src/sandbox-templates.tscode/frameworks/ember/src/client/docs/config.tscode/frameworks/ember/build-config.ts
code/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
code/**/*.{ts,tsx,js,jsx,mjs}: Use server-side logger from 'storybook/internal/node-logger' for Node.js code
Use client-side logger from 'storybook/internal/client-logger' for browser code
Do not use console.log, console.warn, or console.error directly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/frameworks/ember/src/client/portable-stories.tscode/frameworks/ember/src/client/types.tscode/lib/create-storybook/src/generators/EMBER/index.tscode/frameworks/ember/src/preset.tscode/frameworks/ember/src/client/render.tscode/frameworks/ember/src/client/config.tscode/frameworks/ember/src/client/public-types.tscode/frameworks/ember/src/client/docs/sourceDecorator.tscode/frameworks/ember/template/cli/ts/Header.stories.tscode/lib/cli-storybook/src/sandbox-templates.tscode/frameworks/ember/src/client/docs/config.tscode/frameworks/ember/build-config.tscode/frameworks/ember/template/components/index.js
code/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Export functions that need to be tested from their modules
Files:
code/frameworks/ember/src/client/portable-stories.tscode/frameworks/ember/src/client/types.tscode/lib/create-storybook/src/generators/EMBER/index.tscode/frameworks/ember/src/preset.tscode/frameworks/ember/src/client/render.tscode/frameworks/ember/src/client/config.tscode/frameworks/ember/src/client/public-types.tscode/frameworks/ember/src/client/docs/sourceDecorator.tscode/frameworks/ember/template/cli/ts/Header.stories.tscode/lib/cli-storybook/src/sandbox-templates.tscode/frameworks/ember/src/client/docs/config.tscode/frameworks/ember/build-config.tscode/frameworks/ember/template/components/index.js
code/**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
code/**/*.{js,jsx,json,html,ts,tsx,mjs}: Run Prettier with --write flag to format code before committing
Run ESLint with yarn lint:js:cmd to check for linting issues and fix errors before committing
Files:
code/frameworks/ember/src/client/portable-stories.tscode/frameworks/ember/src/client/types.tscode/frameworks/ember/tsconfig.jsoncode/lib/create-storybook/src/generators/EMBER/index.tscode/frameworks/ember/src/preset.tscode/frameworks/ember/src/client/render.tscode/frameworks/ember/src/client/config.tscode/frameworks/ember/src/client/public-types.tscode/frameworks/ember/src/client/docs/sourceDecorator.tscode/frameworks/ember/template/cli/ts/Header.stories.tscode/lib/cli-storybook/src/sandbox-templates.tscode/frameworks/ember/src/client/docs/config.tscode/frameworks/ember/build-config.tscode/frameworks/ember/template/components/index.js
🧠 Learnings (20)
📓 Common learnings
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.
📚 Learning: 2025-11-05T09:36:55.944Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Tabs/Tabs.stories.tsx:222-227
Timestamp: 2025-11-05T09:36:55.944Z
Learning: Repo: storybookjs/storybook PR: 32458 — In code/core/src/components/components/Button/Button.tsx (React/TypeScript), ButtonProps includes ariaLabel?: string | false and the component maps it to the DOM aria-label. Convention: ariaLabel is mandatory on all Button usages — provide a descriptive string for icon-only buttons; set ariaLabel=false when the button’s children already serve as the accessible name. Do not suggest using a raw aria-label prop on Button call sites.
Applied to files:
code/frameworks/ember/template/components/button.gjscode/frameworks/ember/template/components/index.js
📚 Learning: 2025-11-28T14:50:24.889Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Applies to code/**/*.{ts,tsx,js,jsx,mjs} : Use client-side logger from 'storybook/internal/client-logger' for browser code
Applied to files:
code/frameworks/ember/src/client/types.tscode/frameworks/ember/tsconfig.jsoncode/frameworks/ember/src/client/render.tscode/frameworks/ember/src/client/public-types.ts
📚 Learning: 2025-11-28T14:50:24.889Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Applies to **/*.{js,jsx,json,html,ts,tsx,mjs} : Use ESLint and Prettier configurations that are enforced in the codebase
Applied to files:
code/frameworks/ember/tsconfig.jsoncode/lib/cli-storybook/src/sandbox-templates.tscode/frameworks/ember/build-config.ts
📚 Learning: 2025-11-28T14:50:24.889Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Applies to **/*.{ts,tsx} : Enable TypeScript strict mode
Applied to files:
code/frameworks/ember/tsconfig.jsoncode/lib/cli-storybook/src/sandbox-templates.tscode/frameworks/ember/build-config.ts
📚 Learning: 2025-11-28T14:50:24.889Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Applies to code/**/*.{ts,tsx,js,jsx,mjs} : Use server-side logger from 'storybook/internal/node-logger' for Node.js code
Applied to files:
code/frameworks/ember/tsconfig.jsoncode/frameworks/ember/src/client/render.ts
📚 Learning: 2025-09-24T09:39:39.233Z
Learnt from: ndelangen
Repo: storybookjs/storybook PR: 32507
File: code/core/src/manager/globals/globals-module-info.ts:25-33
Timestamp: 2025-09-24T09:39:39.233Z
Learning: In Storybook, storybook/actions/decorator is a preview-only entrypoint and should not be included in manager globals configuration. The duplicatedKeys array in code/core/src/manager/globals/globals-module-info.ts is specifically for manager-side externalization, not preview entrypoints.
Applied to files:
code/lib/create-storybook/src/generators/EMBER/index.tscode/frameworks/ember/src/preset.tscode/frameworks/ember/src/client/public-types.tscode/frameworks/ember/src/client/docs/sourceDecorator.tscode/frameworks/ember/src/client/docs/config.tscode/frameworks/ember/template/components/index.js
📚 Learning: 2025-11-05T09:38:47.712Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.
Applied to files:
code/frameworks/ember/src/client/render.tscode/frameworks/ember/src/client/docs/sourceDecorator.tscode/lib/cli-storybook/src/sandbox-templates.tscode/frameworks/ember/template/components/index.js
📚 Learning: 2025-11-05T09:37:25.920Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/tooltip/WithTooltip.tsx:54-96
Timestamp: 2025-11-05T09:37:25.920Z
Learning: Repo: storybookjs/storybook — In code/core/src/components/components/tooltip/WithTooltip.tsx, the legacy WithTooltip implementation is intentionally reintroduced for backward compatibility and is deprecated; maintainers (per Sidnioulz) do not want maintenance or improvements on it. Prefer WithTooltipNew/Popover; avoid suggesting changes to WithTooltip.* going forward.
Applied to files:
code/frameworks/ember/src/client/render.tscode/frameworks/ember/src/client/docs/sourceDecorator.tscode/frameworks/ember/template/components/index.js
📚 Learning: 2025-11-28T14:50:24.889Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Applies to code/**/*.{ts,tsx,js,jsx,mjs} : Do not use console.log, console.warn, or console.error directly unless in isolated files where importing loggers would significantly increase bundle size
Applied to files:
code/frameworks/ember/src/client/render.ts
📚 Learning: 2025-09-24T13:04:58.631Z
Learnt from: cylewaitforit
Repo: storybookjs/storybook PR: 31965
File: code/lib/eslint-plugin/src/rules/only-csf3.ts:31-33
Timestamp: 2025-09-24T13:04:58.631Z
Learning: The Storybook ESLint plugin supports ESLint v8.57+ where context.sourceCode is already available as a property since it was introduced in v8.40.0, so no fallback to context.getSourceCode() is needed in rules.
Applied to files:
code/frameworks/ember/src/client/docs/sourceDecorator.ts
📚 Learning: 2025-11-28T14:50:24.889Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Follow existing patterns and conventions in the Storybook codebase
Applied to files:
code/frameworks/ember/template/cli/ts/Header.stories.tscode/lib/cli-storybook/src/sandbox-templates.tscode/frameworks/ember/template/components/index.js
📚 Learning: 2025-09-29T13:20:23.346Z
Learnt from: mrginglymus
Repo: storybookjs/storybook PR: 32556
File: code/core/package.json:309-313
Timestamp: 2025-09-29T13:20:23.346Z
Learning: The `fast-printf` dependency in Storybook's core package is bundled into the final distribution during the build process, so it should remain in devDependencies rather than being moved to dependencies, following the same pattern as other bundled dependencies like `open`.
Applied to files:
code/lib/cli-storybook/src/sandbox-templates.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mocking only a subset of required dependencies in Vitest tests
Applied to files:
code/lib/cli-storybook/src/sandbox-templates.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Document complex mock behaviors in Vitest tests
Applied to files:
code/lib/cli-storybook/src/sandbox-templates.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Implement mock behaviors in `beforeEach` blocks in Vitest tests
Applied to files:
code/lib/cli-storybook/src/sandbox-templates.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mocking without the `spy: true` option in Vitest tests
Applied to files:
code/lib/cli-storybook/src/sandbox-templates.ts
📚 Learning: 2025-11-28T14:50:24.889Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Applies to code/**/*.{ts,tsx,js,jsx} : Export functions that need to be tested from their modules
Applied to files:
code/frameworks/ember/build-config.ts
📚 Learning: 2025-09-18T20:51:06.618Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/viewport/components/Tool.tsx:38-39
Timestamp: 2025-09-18T20:51:06.618Z
Learning: The useGlobals hook from storybook/manager-api returns a tuple where the third element (storyGlobals) is typed as Globals, not Globals | undefined. This means TypeScript guarantees it's always defined, making the `in` operator safe to use without additional null checks.
Applied to files:
code/frameworks/ember/template/components/index.js
📚 Learning: 2025-09-18T20:51:06.618Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/viewport/components/Tool.tsx:38-39
Timestamp: 2025-09-18T20:51:06.618Z
Learning: In viewport tool code, when using the `useGlobals` hook from storybook/manager-api, the third returned value `storyGlobals` is guaranteed by TypeScript to be defined (not undefined/null), making the `in` operator safe to use without additional null checks.
Applied to files:
code/frameworks/ember/template/components/index.js
🧬 Code graph analysis (6)
code/frameworks/ember/src/client/portable-stories.ts (3)
code/core/src/preview-api/modules/preview-web/docs-context/DocsContext.ts (1)
projectAnnotations(109-115)code/frameworks/ember/src/client/public-types.ts (1)
EmberRenderer(16-16)code/frameworks/ember/src/client/types.ts (1)
EmberRenderer(12-20)
code/frameworks/ember/src/client/types.ts (1)
code/frameworks/ember/src/client/public-types.ts (3)
EmberRenderer(16-16)Parameters(15-15)StoryContext(41-41)
code/frameworks/ember/src/preset.ts (4)
code/core/src/types/modules/core-common.ts (1)
PresetProperty(657-659)code/builders/builder-vite/src/index.ts (1)
withoutVitePlugins(14-14)code/frameworks/ember/src/types.ts (1)
StorybookConfig(33-38)code/frameworks/ember/src/node/indexer.ts (1)
emberIndexer(19-25)
code/frameworks/ember/src/client/config.ts (3)
code/frameworks/ember/src/client/docs/config.ts (1)
parameters(6-13)code/frameworks/ember/src/client/public-types.ts (1)
Parameters(15-15)code/frameworks/ember/src/client/types.ts (1)
Parameters(22-25)
code/frameworks/ember/template/cli/ts/Header.stories.ts (2)
code/frameworks/ember/template/cli/js/Header.stories.js (1)
meta(5-17)code/frameworks/ember/src/client/public-types.ts (2)
Meta(23-23)StoryObj(37-37)
code/frameworks/ember/src/client/docs/config.ts (4)
code/frameworks/ember/src/client/config.ts (1)
parameters(6-11)code/frameworks/ember/src/client/public-types.ts (1)
Parameters(15-15)code/frameworks/ember/src/client/types.ts (1)
Parameters(22-25)code/frameworks/ember/src/client/docs/sourceDecorator.ts (1)
sourceDecorator(23-41)
🪛 Biome (2.1.2)
code/frameworks/ember/template/components/index.js
[error] 1-1: Do not shadow the global "globalThis" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: normal
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (10)
code/frameworks/ember/template/cli/ts/Header.stories.ts (3)
24-32: LoggedIn/LoggedOut story definitions are clean and consistent with the JS template story
Args shape reads well, and keepingLoggedOutempty is a nice baseline case.
5-23: The.gtsimport pattern is correctly implemented and consistently used.This is the right direction for strict, type-safe story templates. The
.gtsfiles are properly resolved throughember-source/typesin the TypeScript configuration, and this pattern is already established across all Ember template stories (Button, Header, Page). No additional ambient type declarations are needed—the setup is correct.
1-5: No action required. The imports in this file are correct and follow Storybook v10 standards.storybook/testis the official consolidated import path for thefnhelper (replacing the legacy@storybook/testpackage), and.gtsimports are fully supported in Ember templates. The file structure and typing viasatisfies Meta<Signature['Args']>are appropriate.code/lib/cli-storybook/src/sandbox-templates.ts (2)
753-768:inDevelopment: trueis now correctly annotated with a TODO (thanks).
988-1007: inDevelopment templates are correctly excluded from CI runs.The concern about
ember/default-tsrunning in CI despite havinginDevelopment: trueis unfounded. Whileember/default-tsdoes appear in thenormalcadence array, it is properly filtered out before execution through theisTaskSkipped()function in./scripts/get-template.ts(line 22), which excludes any template whereinDevelopment !== true. Additionally,./scripts/create-nx-sandbox-projects.tsexplicitly checks!value.inDevelopmentwhen assigning CI tags. The templates are kept in cadence arrays for tracking purposes but are excluded from actual CI runs via these filters.code/frameworks/ember/src/preset.ts (1)
55-64: No issue found.import.meta.resolve()returns a file:// URL that is fully compatible with the preset system—importModule()passes the value directly to Node.js's dynamicimport(), which accepts file:// URLs natively. This pattern is already in use across multiple frameworks (Angular, Server Webpack5, React Vite) without runtime issues.code/lib/create-storybook/src/generators/EMBER/index.ts (1)
12-16: Verify generator output still serves Ember app assets correctly after removingstaticDir/builder override.
Addinggts/gjsextensions looks right, but please confirm the generated config doesn’t regress typical Ember asset loading (historicallydist/) and that the intended builder (Vite) is selected end-to-end.code/frameworks/ember/src/client/render.ts (1)
88-95: ConfirmtrackedObjectobserves deletes; otherwise prefer settingundefined.
If deletes aren’t tracked, removed args won’t trigger a rerender. Worth confirming against Ember’strackedObjectbehavior.code/frameworks/ember/src/client/public-types.ts (1)
1-42: Public typing surface looks consistent and appropriately scoped.
Nice to see the EmberRenderer-bound CSF aliases exposed in one place.code/frameworks/ember/build-config.ts (1)
5-22: Build entries addition looks right; verify exports map + d.ts output align.
Please confirmcode/frameworks/ember/package.jsonexports include./client,./client/config, and./client/docs/configand that the build produces the expected.d.tsfor these browser entries.
| export const sourceDecorator: DecoratorFunction<EmberRenderer> = (storyFn, context) => { | ||
| const source = useRef<string | undefined>(undefined); | ||
| const story = storyFn(); | ||
|
|
||
| useEffect(() => { | ||
| const renderedForSource = context?.parameters.docs?.source?.excludeDecorators | ||
| ? (context.originalStoryFn as StoryFn)(context.args, context) | ||
| : story; | ||
|
|
||
| if (!skipSourceRender(context)) { | ||
| const code = | ||
| generateGlimmerSource(renderedForSource, context.args, context.argTypes) ?? undefined; | ||
| emitTransformCode(code, context); | ||
| source.current = code; | ||
| } | ||
| }); | ||
|
|
||
| return story; | ||
| }; |
There was a problem hiding this comment.
Add dependency array and cache guard to prevent emitting transformed source on every render.
The useEffect() hook currently lacks a dependency array, causing it to run after every render and calling emitTransformCode unnecessarily. The source.current cache is assigned but never checked before emitting. This differs from the React and other framework implementations, which properly track dependencies and avoid redundant channel emissions.
export const sourceDecorator: DecoratorFunction<EmberRenderer> = (storyFn, context) => {
const source = useRef<string | undefined>(undefined);
const story = storyFn();
useEffect(() => {
const renderedForSource = context?.parameters.docs?.source?.excludeDecorators
? (context.originalStoryFn as StoryFn)(context.args, context)
: story;
- if (!skipSourceRender(context)) {
- const code =
- generateGlimmerSource(renderedForSource, context.args, context.argTypes) ?? undefined;
- emitTransformCode(code, context);
- source.current = code;
- }
- });
+ if (skipSourceRender(context)) return;
+
+ const code =
+ generateGlimmerSource(renderedForSource as any, context.args, context.argTypes) ?? undefined;
+
+ if (code !== source.current) {
+ emitTransformCode(code, context);
+ source.current = code;
+ }
+ }, [
+ context.id,
+ context.args,
+ context.argTypes,
+ context.parameters.docs?.source?.excludeDecorators,
+ context.parameters.docs?.source?.type,
+ context.parameters.docs?.source?.code,
+ context.parameters.__isArgsStory,
+ ]);🤖 Prompt for AI Agents
In code/frameworks/ember/src/client/docs/sourceDecorator.ts around lines 23 to
41, update the useEffect to include an appropriate dependency array and a cache
guard so transformed source is only emitted when it changes: compute the new
code as before, then compare it to source.current and only call
emitTransformCode and update source.current when code !== source.current;
finally, add a dependency array that covers the values that can change (for
example: context.args, context.argTypes,
context.parameters?.docs?.source?.excludeDecorators, context.originalStoryFn,
and story) so the effect runs only when relevant inputs change.
| export function generateGlimmerSource( | ||
| component: object & { name?: string }, | ||
| args: Args, | ||
| argTypes: ArgTypes | ||
| ): string | null { | ||
| const name = component.name; | ||
| if (!name) { | ||
| return null; | ||
| } | ||
|
|
||
| const propsArray = Object.entries(args) | ||
| .map(([k, v]) => toArgument(k, v, argTypes)) | ||
| .filter((p) => p); | ||
|
|
||
| if (propsArray.length === 0) { | ||
| return `<${name} />`; | ||
| } else if (propsArray.length > 3) { | ||
| return `<${name}\n ${propsArray.join('\n ')}\n/>`; | ||
| } | ||
| return `<${name} ${propsArray.join(' ')} />`; | ||
| } |
There was a problem hiding this comment.
generateGlimmerSource() likely reads the wrong “component” shape for Ember stories.
In this renderer, the story output is typically a render result object (e.g. { Component, args }), not the component itself—so component.name will often be undefined and source won’t render.
export function generateGlimmerSource(
- component: object & { name?: string },
+ rendered: any,
args: Args,
argTypes: ArgTypes
): string | null {
- const name = component.name;
+ const component = rendered?.Component ?? rendered;
+ const name: string | undefined = component?.name;
if (!name) {
return null;
}
@@
}🤖 Prompt for AI Agents
In code/frameworks/ember/src/client/docs/sourceDecorator.ts around lines 43 to
63, generateGlimmerSource assumes the passed "component" is the component itself
and reads component.name, but Ember stories pass a render result object (e.g. {
Component, args }), so component.name is often undefined; change the name
extraction to handle both shapes by checking for a Component property and using
Component.name when present (falling back to component.name), update the
function/type signature accordingly, and keep the existing null-return behavior
if no name can be resolved.
| import * as INTERNAL_DEFAULT_PROJECT_ANNOTATIONS from './render'; | ||
| import type { EmberRenderer } from './types'; | ||
|
|
||
| /** | ||
| * Function that sets the globalConfig of your storybook. The global config is the preview module of | ||
| * your .storybook folder. | ||
| * | ||
| * It should be run a single time, so that your global config (e.g. decorators) is applied to your | ||
| * stories when using `composeStories` or `composeStory`. | ||
| * | ||
| * Example: | ||
| * | ||
| * ```jsx | ||
| * // setup-file.js | ||
| * import { setProjectAnnotations } from '@storybook/preact'; | ||
| * import projectAnnotations from './.storybook/preview'; | ||
| * | ||
| * setProjectAnnotations(projectAnnotations); | ||
| * ``` | ||
| * | ||
| * @param projectAnnotations - E.g. (import projectAnnotations from '../.storybook/preview') | ||
| */ | ||
| export function setProjectAnnotations( | ||
| projectAnnotations: | ||
| | NamedOrDefaultProjectAnnotations<any> | ||
| | NamedOrDefaultProjectAnnotations<any>[] | ||
| ): NormalizedProjectAnnotations<EmberRenderer> { | ||
| setDefaultProjectAnnotations(INTERNAL_DEFAULT_PROJECT_ANNOTATIONS); | ||
| return originalSetProjectAnnotations( | ||
| projectAnnotations | ||
| ) as NormalizedProjectAnnotations<EmberRenderer>; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# First, check if the file exists and get its current state
echo "=== Checking portable-stories.ts file existence and content ==="
wc -l code/frameworks/ember/src/client/portable-stories.ts
# Inspect what ./render exports to ensure it's suitable as default project annotations
echo -e "\n=== Inspecting render.ts exports ==="
rg -n --type=ts "^export\s+(const|function|type|interface|class)\s+" code/frameworks/ember/src/client/render.ts
# Also check for namespace exports and default exports
echo -e "\n=== Checking for export patterns in render.ts ==="
rg -n "^export\s+" code/frameworks/ember/src/client/render.ts | head -20
# Check the entire render.ts file to understand its structure
echo -e "\n=== Checking render.ts file size and structure ==="
wc -l code/frameworks/ember/src/client/render.tsRepository: storybookjs/storybook
Length of output: 560
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check the actual imports and type definitions in portable-stories.ts
echo "=== Reading portable-stories.ts to see current state ==="
cat -n code/frameworks/ember/src/client/portable-stories.ts | head -50
# Look for setDefaultProjectAnnotations import
echo -e "\n=== Checking for setDefaultProjectAnnotations import ==="
rg -n "setDefaultProjectAnnotations|originalSetProjectAnnotations" code/frameworks/ember/src/client/portable-stories.tsRepository: storybookjs/storybook
Length of output: 2051
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check the render.ts file content to understand what gets exported as INTERNAL_DEFAULT_PROJECT_ANNOTATIONS
echo "=== Reading render.ts file content ==="
cat -n code/frameworks/ember/src/client/render.tsRepository: storybookjs/storybook
Length of output: 3343
🏁 Script executed:
#!/bin/bash
set -eulo pipefail
# Search for NamedOrDefaultProjectAnnotations type definition
echo "=== Searching for NamedOrDefaultProjectAnnotations type definition ==="
rg -n "NamedOrDefaultProjectAnnotations" code/frameworks/ember/src/client/ -A 3
# Also search in common locations
echo -e "\n=== Searching in storybook internal types ==="
fd "types.ts" code/ | xargs rg -l "NamedOrDefaultProjectAnnotations" 2>/dev/null || echo "Not found with fd approach"
# Direct search
echo -e "\n=== Direct search for type definition ==="
rg "type NamedOrDefaultProjectAnnotations|interface NamedOrDefaultProjectAnnotations" -A 3Repository: storybookjs/storybook
Length of output: 1622
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find and examine setDefaultProjectAnnotations signature
echo "=== Searching for setDefaultProjectAnnotations definition ==="
rg -n "setDefaultProjectAnnotations" code/core/src --type=ts -A 5 | head -40
# Look at the ProjectAnnotations type to understand what should be passed
echo -e "\n=== Searching for ProjectAnnotations type definition ==="
rg -n "type ProjectAnnotations|interface ProjectAnnotations" code/core/src --type=ts -A 10 | head -50Repository: storybookjs/storybook
Length of output: 3215
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Compare how other frameworks (like react) handle portable-stories
echo "=== Checking react framework's portable-stories ==="
if [ -f "code/frameworks/react/src/client/portable-stories.ts" ]; then
cat -n code/frameworks/react/src/client/portable-stories.ts | head -50
fi
# Check preact too
echo -e "\n=== Checking preact framework's portable-stories ==="
if [ -f "code/frameworks/preact/src/client/portable-stories.ts" ]; then
cat -n code/frameworks/preact/src/client/portable-stories.ts | head -50
fiRepository: storybookjs/storybook
Length of output: 170
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for other frameworks' render.ts to see what they export
echo "=== Checking react framework's render.ts exports ==="
if [ -f "code/frameworks/react/src/client/render.ts" ]; then
rg -n "^export\s+" code/frameworks/react/src/client/render.ts
fi
echo -e "\n=== Checking preact framework's render.ts exports ==="
if [ -f "code/frameworks/preact/src/client/render.ts" ]; then
rg -n "^export\s+" code/frameworks/preact/src/client/render.ts
fi
echo -e "\n=== Checking vue framework's render.ts exports ==="
if [ -f "code/frameworks/vue3/src/client/render.ts" ]; then
rg -n "^export\s+" code/frameworks/vue3/src/client/render.ts
fiRepository: storybookjs/storybook
Length of output: 224
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# List all client directory structures
echo "=== Framework structure comparison ==="
for dir in code/frameworks/*/src/client/; do
echo "Framework: $(basename $(dirname $(dirname $(dirname "$dir"))))"
ls -1 "$dir" | grep -E "(render|portable-stories|preview)" || echo " (no matching files)"
doneRepository: storybookjs/storybook
Length of output: 223
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for any explicit ProjectAnnotations assignments or exports in ember
echo "=== Searching for ProjectAnnotations in ember framework ==="
rg -n "ProjectAnnotations" code/frameworks/ember/src/client/ --type=ts
# Check if there's a preview or index file
echo -e "\n=== Files in ember client directory ==="
ls -la code/frameworks/ember/src/client/
# Look for any setDefaultProjectAnnotations calls to understand the pattern
echo -e "\n=== All setDefaultProjectAnnotations usage patterns ==="
rg -B 5 "setDefaultProjectAnnotations\(" code/frameworks/ --type=tsRepository: storybookjs/storybook
Length of output: 5235
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check the actual behavior - what does renderToCanvas signature expect?
echo "=== Checking RenderToCanvas type definition ==="
rg -n "type RenderToCanvas|interface RenderToCanvas" code/core/src --type=ts -A 5
# Check if render function is supposed to be in ProjectAnnotations
echo -e "\n=== Checking ArgsStoryFn usage ==="
rg -n "ArgsStoryFn" code/core/src/types --type=ts -A 3 | head -30Repository: storybookjs/storybook
Length of output: 2471
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check if INTERNAL_DEFAULT_PROJECT_ANNOTATIONS needs type casting
echo "=== Checking if similar patterns exist in nextjs-vite ==="
cat -n code/frameworks/nextjs-vite/src/portable-stories.ts | head -50
# Check if there's a type definition or explicit casting
echo -e "\n=== Checking nextjs-vite render.ts for comparison ==="
if [ -f "code/frameworks/nextjs-vite/src/render.ts" ]; then
head -30 code/frameworks/nextjs-vite/src/render.ts
fiRepository: storybookjs/storybook
Length of output: 2251
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify types are consistent - check BaseProjectAnnotations
echo "=== Checking BaseProjectAnnotations ==="
rg -n "interface BaseProjectAnnotations|type BaseProjectAnnotations" code/core/src --type=ts -A 15 | head -40Repository: storybookjs/storybook
Length of output: 104
Fix the JSDoc example + tighten types; verify setDefaultProjectAnnotations receives valid project annotations.
- JSDoc shows
@storybook/preact(line 25) instead of@storybook/ember. - Function parameter types use
anyinstead ofEmberRenderer, weakening type safety in a public API. import * as INTERNAL_DEFAULT_PROJECT_ANNOTATIONS from './render'creates a namespace containingrenderandrenderToCanvasfunctions, butrenderis not part of theProjectAnnotationsinterface. The namespace should either be explicitly typed asProjectAnnotations<EmberRenderer>or restructured to avoid passing non-annotation symbols tosetDefaultProjectAnnotations.
import type {
NamedOrDefaultProjectAnnotations,
NormalizedProjectAnnotations,
} from 'storybook/internal/types';
import {
setProjectAnnotations as originalSetProjectAnnotations,
setDefaultProjectAnnotations,
} from 'storybook/preview-api';
import * as INTERNAL_DEFAULT_PROJECT_ANNOTATIONS from './render';
import type { EmberRenderer } from './types';
/**
* Function that sets the globalConfig of your storybook. The global config is the preview module of
* your .storybook folder.
*
* It should be run a single time, so that your global config (e.g. decorators) is applied to your
* stories when using `composeStories` or `composeStory`.
*
* Example:
*
* ```jsx
* // setup-file.js
- * import { setProjectAnnotations } from '@storybook/preact';
+ * import { setProjectAnnotations } from '@storybook/ember';
* import projectAnnotations from './.storybook/preview';
*
* setProjectAnnotations(projectAnnotations);
* ```
*
* @param projectAnnotations - E.g. (import projectAnnotations from '../.storybook/preview')
*/
export function setProjectAnnotations(
projectAnnotations:
- | NamedOrDefaultProjectAnnotations<any>
- | NamedOrDefaultProjectAnnotations<any>[]
+ | NamedOrDefaultProjectAnnotations<EmberRenderer>
+ | NamedOrDefaultProjectAnnotations<EmberRenderer>[]
): NormalizedProjectAnnotations<EmberRenderer> {
setDefaultProjectAnnotations(INTERNAL_DEFAULT_PROJECT_ANNOTATIONS);
return originalSetProjectAnnotations(
projectAnnotations
) as NormalizedProjectAnnotations<EmberRenderer>;
}🤖 Prompt for AI Agents
In code/frameworks/ember/src/client/portable-stories.ts around lines 11 to 41,
update the JSDoc example to import from '@storybook/ember' instead of
'@storybook/preact', tighten the function parameter types by replacing
NamedOrDefaultProjectAnnotations<any> with
NamedOrDefaultProjectAnnotations<EmberRenderer> (and the array variant), and
ensure setDefaultProjectAnnotations receives a valid
ProjectAnnotations<EmberRenderer> payload by either exporting/constructing an
explicit ProjectAnnotations<EmberRenderer> object from ./render or by
narrowing/casting INTERNAL_DEFAULT_PROJECT_ANNOTATIONS to
ProjectAnnotations<EmberRenderer> (remove or exclude non-annotation exports like
render/renderToCanvas before passing to setDefaultProjectAnnotations).
| export const render: ArgsStoryFn<EmberRenderer> = (args, context) => { | ||
| const { id, component: Component } = context; | ||
|
|
||
| if (typeof Component === 'function') { | ||
| return { Component, args }; | ||
| } else if (typeof Component === 'object') { | ||
| return { Component, args }; | ||
| } | ||
|
|
||
| throw new Error( | ||
| `Unable to render story ${id} as the component annotation is missing from the default export` | ||
| ); | ||
| }; |
There was a problem hiding this comment.
Tweak error message: not always “missing from default export”.
If component exists but is an unexpected type, the current message is misleading; consider mentioning “missing or invalid” to aid debugging.
🤖 Prompt for AI Agents
In code/frameworks/ember/src/client/render.ts around lines 10 to 22, the thrown
error message only says the component annotation is "missing from the default
export" which is misleading if a component exists but is the wrong type; change
the thrown Error text to indicate the component is "missing or invalid" and
include contextual info (e.g., story id and the found typeof Component) so
consumers see whether the annotation was absent or of an unexpected type.
| const application: Application | undefined = storyContext.parameters['owner'] | ||
| ? storyContext.parameters['owner'].create({ | ||
| autoboot: false, | ||
| rootElement: 'body', | ||
| }) | ||
| : undefined; | ||
|
|
||
| const trackedArgs = trackedObject({ ...args }); | ||
| const result = renderComponent(Component, { | ||
| args: trackedArgs, | ||
| into: canvasElement, | ||
| owner: application ? application.buildInstance() : undefined, | ||
| }); | ||
|
|
||
| contexts.set(canvasElement, { application, renderer: result, args: trackedArgs }); | ||
|
|
There was a problem hiding this comment.
Potential owner instance leak: application.buildInstance() result isn’t tracked/destroyed.
buildInstance() likely creates an application instance that should be retained and destroyed on unmount; currently only application is stored. Please verify the expected lifecycle for Ember 6.8 + renderComponent.
import type Application from '@ember/application';
+import type ApplicationInstance from '@ember/application/instance';
@@
const contexts = new Map<
EmberRenderer['canvasElement'],
{
application: Application | undefined;
+ instance: ApplicationInstance | undefined;
renderer: RenderResult;
args: Args;
}
>();
@@
- const application: Application | undefined = storyContext.parameters['owner']
+ const application: Application | undefined = storyContext.parameters['owner']
? storyContext.parameters['owner'].create({
autoboot: false,
rootElement: 'body',
})
: undefined;
+ const instance = application ? application.buildInstance() : undefined;
@@
const result = renderComponent(Component, {
args: trackedArgs,
into: canvasElement,
- owner: application ? application.buildInstance() : undefined,
+ owner: instance,
});
- contexts.set(canvasElement, { application, renderer: result, args: trackedArgs });
+ contexts.set(canvasElement, { application, instance, renderer: result, args: trackedArgs });(And in unmount(), destroy context.instance as well.)
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In code/frameworks/ember/src/client/render.ts around lines 65 to 80, the code
calls application.buildInstance() but only stores the parent application in
contexts, so the created application instance is not tracked or destroyed;
capture the buildInstance() return value and store it (e.g., context.instance)
alongside application and renderer so it can be cleanly destroyed on unmount,
and update unmount() to call destroy() on both context.instance (if present) and
context.application to avoid leaking Ember instances; ensure null/undefined
checks and follow Ember 6.8 renderComponent lifecycle expectations when
destroying both objects.
| export interface EmberRenderer extends WebRenderer { | ||
| // We are omitting props, as we don't use it internally, and more importantly, it completely changes the assignability of meta.component. | ||
| // Try not omitting, and check the type errros in the test file, if you want to learn more. | ||
| component: object; | ||
| storyResult: { | ||
| Component: object; | ||
| args: Record<string, unknown>; | ||
| }; | ||
| } |
There was a problem hiding this comment.
Fix typo in comment; consider unknown instead of object for opaque component types.
- Typo: “type errros” (Line 14).
- If you just need an opaque “some Ember component-ish thing”,
unknowntypically composes better thanobject(without making it indexable likeany).
- // Try not omitting, and check the type errros in the test file, if you want to learn more.
+ // Try not omitting, and check the type errors in the test file, if you want to learn more. export interface EmberRenderer extends WebRenderer {
// ...
- component: object;
+ component: unknown;
storyResult: {
- Component: object;
+ Component: unknown;
args: Record<string, unknown>;
};
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export interface EmberRenderer extends WebRenderer { | |
| // We are omitting props, as we don't use it internally, and more importantly, it completely changes the assignability of meta.component. | |
| // Try not omitting, and check the type errros in the test file, if you want to learn more. | |
| component: object; | |
| storyResult: { | |
| Component: object; | |
| args: Record<string, unknown>; | |
| }; | |
| } | |
| export interface EmberRenderer extends WebRenderer { | |
| // We are omitting props, as we don't use it internally, and more importantly, it completely changes the assignability of meta.component. | |
| // Try not omitting, and check the type errors in the test file, if you want to learn more. | |
| component: unknown; | |
| storyResult: { | |
| Component: unknown; | |
| args: Record<string, unknown>; | |
| }; | |
| } |
🤖 Prompt for AI Agents
In code/frameworks/ember/src/client/types.ts around lines 12 to 20, fix the typo
in the comment (“type errros” → “type errors”) and replace the opaque component
types declared as object with unknown (i.e., change component: object to
component: unknown and Component: object to Component: unknown, and keep args as
Record<string, unknown>); update the comment text accordingly to reference “type
errors” and note that unknown composes better for opaque Ember component types.
| export const previewAnnotations: PresetProperty<'previewAnnotations'> = async ( | ||
| entries = [], | ||
| options | ||
| ) => { | ||
| const config = fileURLToPath(import.meta.resolve('@storybook/ember/client/config')); | ||
| const annotations = [...entries, config]; | ||
|
|
||
| export const webpackFinal: StorybookConfig['webpackFinal'] = async (baseConfig, options) => { | ||
| const { virtualModules } = await getVirtualModules(options); | ||
| const docsConfig = await options.presets.apply('docs', {}, options); | ||
| const docsEnabled = Object.keys(docsConfig).length > 0; | ||
| if (docsEnabled) { | ||
| const docsConfigPath = fileURLToPath( | ||
| import.meta.resolve('@storybook/ember/client/docs/config') | ||
| ); | ||
| annotations.push(docsConfigPath); | ||
| } | ||
|
|
||
| const babelOptions = await options.presets.apply('babel', {}, options); | ||
| const typescriptOptions = await options.presets.apply('typescript', {}, options); | ||
| return annotations; | ||
| }; |
There was a problem hiding this comment.
Harden docs enablement detection (avoid assuming docsConfig is an object).
If options.presets.apply('docs', ...) can yield false (or something non-object), Object.keys(docsConfig) (Line 20) will throw.
- const docsConfig = await options.presets.apply('docs', {}, options);
- const docsEnabled = Object.keys(docsConfig).length > 0;
+ const docsConfig = await options.presets.apply('docs', {}, options);
+ const docsEnabled =
+ !!docsConfig && typeof docsConfig === 'object' && Object.keys(docsConfig).length > 0;🤖 Prompt for AI Agents
In code/frameworks/ember/src/preset.ts around lines 12 to 29, the code assumes
docsConfig is always an object and calls Object.keys(docsConfig) which will
throw if presets.apply('docs', ...) returns false or a non-object; update the
docsEnabled check to first handle the boolean-true case and guard against
non-object values (e.g. docsEnabled = docsConfig === true || (docsConfig &&
typeof docsConfig === 'object' && Object.keys(docsConfig).length > 0)), so only
call Object.keys when docsConfig is a non-null object and treat a plain true as
enabling docs.
| import {on} from '@ember/modifier'; | ||
|
|
||
| <template> | ||
| <button {{on "click" @onClick}}>{{@label}}</button> | ||
| </template> |
There was a problem hiding this comment.
Set type="button" to prevent accidental form submissions.
Right now the button defaults to type="submit" when nested in a <form> (Line 4). Safer default for a generic UI button is type="button".
<template>
- <button {{on "click" @onClick}}>{{@label}}</button>
+ <button type="button" {{on "click" @onClick}}>{{@label}}</button>
</template>🤖 Prompt for AI Agents
In code/frameworks/ember/template/components/button.gjs around lines 1 to 5, the
<button> element lacks an explicit type attribute and will default to
type="submit" when inside a form; update the template to set type="button" on
the button element (i.e., add type="button" alongside the existing {{on "click"
@onClick}}) so the component behaves as a generic UI button and does not trigger
form submissions.
| "compilerOptions": { | ||
| "baseUrl": ".", | ||
| "paths": { | ||
| "storybook/internal/*": ["../../lib/cli/core/*"] | ||
| }, | ||
| "rootDir": "./src", | ||
| "types": ["ember-source/types"] | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n code/frameworks/ember/tsconfig.jsonRepository: storybookjs/storybook
Length of output: 399
🏁 Script executed:
cat -n code/tsconfig.jsonRepository: storybookjs/storybook
Length of output: 891
🏁 Script executed:
fd -type d '^core$' code/lib/cliRepository: storybookjs/storybook
Length of output: 235
🏁 Script executed:
ls -la code/lib/cli/ 2>/dev/null | head -20 || echo "Directory may not exist or be empty"Repository: storybookjs/storybook
Length of output: 47
🏁 Script executed:
fd -type d 'core' code/lib/cli/Repository: storybookjs/storybook
Length of output: 235
🏁 Script executed:
ls -la code/lib/cli/core/ 2>/dev/null || echo "core directory not found in lib/cli"Repository: storybookjs/storybook
Length of output: 100
🏁 Script executed:
ls -la code/lib/cli/Repository: storybookjs/storybook
Length of output: 125
🏁 Script executed:
find code/lib -maxdepth 3 -type d -name '*cli*' -o -type d -name '*core*' | head -20Repository: storybookjs/storybook
Length of output: 125
🏁 Script executed:
rg 'storybook/internal' code/frameworks/ember/ --type ts --type tsx --type jsRepository: storybookjs/storybook
Length of output: 92
🏁 Script executed:
find code/lib -maxdepth 2 -type d | grep -E '(cli|core)' | sortRepository: storybookjs/storybook
Length of output: 206
🏁 Script executed:
rg 'storybook/internal' code/frameworks/ember/Repository: storybookjs/storybook
Length of output: 1961
🏁 Script executed:
rg 'storybook/internal' code/ --max-count 5Repository: storybookjs/storybook
Length of output: 50378
🏁 Script executed:
find code -type d -name 'types' | head -5Repository: storybookjs/storybook
Length of output: 185
🏁 Script executed:
find code -type f -path '*/internal/*' | grep -E '(types|docs-tools|csf)' | head -10Repository: storybookjs/storybook
Length of output: 47
🏁 Script executed:
ls -la code/core/src/ | head -20Repository: storybookjs/storybook
Length of output: 1217
🏁 Script executed:
ls -la code/core/src/cli/Repository: storybookjs/storybook
Length of output: 1380
🏁 Script executed:
find code/core/src -maxdepth 1 -type d | sortRepository: storybookjs/storybook
Length of output: 946
Fix the paths alias - the target directory does not exist.
The path alias "storybook/internal/*": ["../../lib/cli/core/*"] is misconfigured. From code/frameworks/ember, this resolves to code/lib/cli/core/*, which does not exist. The actual internal modules are in code/core/src/. Update the alias to ["../../core/src/*"] (or a more specific target like ["../../core/src/cli/*"] if targeting a specific module).
Strict mode is correctly inherited from the parent code/tsconfig.json which sets "strict": true, so that requirement is already satisfied.
🤖 Prompt for AI Agents
In code/frameworks/ember/tsconfig.json around lines 3 to 10 the "paths" alias
points "storybook/internal/*" to "../../lib/cli/core/*" which resolves to a
non-existent code/lib/cli/core/*; change the target to the actual location under
code/core/src by replacing the mapping with ["../../core/src/*"] (or
["../../core/src/cli/*"] if you only want CLI modules) so TypeScript path
resolution points to the real source files.
| 'ember/default-js': { | ||
| name: 'Ember Latest (Vite | JavaScript)', | ||
| script: | ||
| 'npx --package ember-cli ember new {{beforeDir}} -yarn --typescript --no-ember-data --no-welcome --skip-git --skip-install && cd {{beforeDir}}', | ||
| expected: { | ||
| framework: '@storybook/ember', | ||
| renderer: '@storybook/ember', | ||
| builder: '@storybook/builder-webpack5', | ||
| builder: '@storybook/builder-vite', | ||
| }, | ||
| // TODO: Still in active development: Missing args extraction and documentation | ||
| inDevelopment: true, | ||
| skipTasks: ['e2e-tests', 'bench', 'vitest-integration'], | ||
| }, |
There was a problem hiding this comment.
Fix ember/default-js creating a TypeScript app despite “JavaScript” label.
On Line 747, the “Ember Latest (Vite | JavaScript)” template passes --typescript, so the generated sandbox won’t match the template’s name and is likely to surprise users (and could affect expectations around type-checking, deps, etc.).
'ember/default-js': {
name: 'Ember Latest (Vite | JavaScript)',
script:
- 'npx --package ember-cli ember new {{beforeDir}} -yarn --typescript --no-ember-data --no-welcome --skip-git --skip-install && cd {{beforeDir}}',
+ 'npx --package ember-cli ember new {{beforeDir}} -yarn --no-ember-data --no-welcome --skip-git --skip-install && cd {{beforeDir}}',
expected: {
framework: '@storybook/ember',
renderer: '@storybook/ember',
builder: '@storybook/builder-vite',
},
// TODO: Still in active development: Missing args extraction and documentation
inDevelopment: true,
skipTasks: ['e2e-tests', 'bench', 'vitest-integration'],
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 'ember/default-js': { | |
| name: 'Ember Latest (Vite | JavaScript)', | |
| script: | |
| 'npx --package ember-cli ember new {{beforeDir}} -yarn --typescript --no-ember-data --no-welcome --skip-git --skip-install && cd {{beforeDir}}', | |
| expected: { | |
| framework: '@storybook/ember', | |
| renderer: '@storybook/ember', | |
| builder: '@storybook/builder-webpack5', | |
| builder: '@storybook/builder-vite', | |
| }, | |
| // TODO: Still in active development: Missing args extraction and documentation | |
| inDevelopment: true, | |
| skipTasks: ['e2e-tests', 'bench', 'vitest-integration'], | |
| }, | |
| 'ember/default-js': { | |
| name: 'Ember Latest (Vite | JavaScript)', | |
| script: | |
| 'npx --package ember-cli ember new {{beforeDir}} -yarn --no-ember-data --no-welcome --skip-git --skip-install && cd {{beforeDir}}', | |
| expected: { | |
| framework: '@storybook/ember', | |
| renderer: '@storybook/ember', | |
| builder: '@storybook/builder-vite', | |
| }, | |
| // TODO: Still in active development: Missing args extraction and documentation | |
| inDevelopment: true, | |
| skipTasks: ['e2e-tests', 'bench', 'vitest-integration'], | |
| }, |
🤖 Prompt for AI Agents
In code/lib/cli-storybook/src/sandbox-templates.ts around lines 744 to 756, the
Ember "JavaScript" template is invoking ember new with the --typescript flag
which creates a TypeScript app; remove the --typescript flag (or replace with
the equivalent JavaScript option if needed) so the generated sandbox is plain
JavaScript and update any related comments/flags if they reference TypeScript to
keep the template consistent with its "JavaScript" label.
What I did
Ember just released Ember 6.8 with Vite as its default build system as well as support for rendering components directly to DOM, as well as passing arguments as an object, which makes it perfect to use to render stories in Storybook. To my understanding the current Ember renderer isn't compatible with Storybook v10 and I've therefor completely replaced it.
The new integration uses Vite which makes it possible to integrate with Storybook v10. The new integration doesn't boot the app by default, the ergonomics of using an app should be explored further.
Supersedes #32790
Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
yarn task --task sandbox --start-from auto --template ember/default-tsDocumentation
MIGRATION.MD
Checklist for Maintainers
When this PR is ready for testing, make sure to add
ci:normal,ci:mergedorci:dailyGH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/lib/cli-storybook/src/sandbox-templates.tsMake sure this PR contains one of the labels below:
Available labels
bug: Internal changes that fixes incorrect behavior.maintenance: User-facing maintenance tasks.dependencies: Upgrading (sometimes downgrading) dependencies.build: Internal-facing build tooling & test updates. Will not show up in release changelog.cleanup: Minor cleanup style change. Will not show up in release changelog.documentation: Documentation only changes. Will not show up in release changelog.feature request: Introducing a new feature.BREAKING CHANGE: Changes that break compatibility in some way with current major version.other: Changes that don't fit in the above categories.🦋 Canary release
This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the
@storybookjs/coreteam here.core team members can create a canary release here or locally with
gh workflow run --repo storybookjs/storybook publish.yml --field pr=<PR_NUMBER>Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.