feat: Add ember-resources integration via resource factories#3
feat: Add ember-resources integration via resource factories#3
Conversation
There was a problem hiding this comment.
Pull request overview
Adds an optional ember-resources integration layer to glimmer-apollo by introducing resource factory entrypoints that wrap existing QueryResource, MutationResource, and SubscriptionResource classes for use in templates and via the @use decorator.
Changes:
- Introduces
queryResource/mutationResource/subscriptionResourceplus curriedcreate*Resourcehelpers underglimmer-apollo/resource-factories. - Exposes the new subpath export and Rollup public entrypoint, with
ember-resourcesas an optional peer dependency. - Adds unit + integration tests and end-user docs for resource factory usage patterns.
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test-app/tests/unit/resource-factory-test.ts | Adds unit coverage for the new resource factories (query/mutation/subscription + curried variants). |
| test-app/tests/unit/query-test.ts | Adds a regression test around refetch() behavior when a query starts skipped. |
| test-app/tests/integration/components/resource-factory-test.gts | Adds rendering tests proving template + @use decorator usage and subscription teardown behavior. |
| test-app/package.json | Adds ember-resources to the test app to exercise the integration. |
| pnpm-lock.yaml | Locks ember-resources dependency resolution. |
| glimmer-apollo/src/resource-factories.ts | Adds the public entrypoint re-exporting the factories from a private module. |
| glimmer-apollo/src/-private/resource-factories.ts | Implements wrapResource + factory APIs using ember-resources and glimmer-apollo Resource classes. |
| glimmer-apollo/rollup.config.mjs | Marks resource-factories.js as a public entrypoint for the addon build. |
| glimmer-apollo/package.json | Adds the ./resource-factories export plus optional peer dep metadata for ember-resources. |
| docs/fetching/resource-factories.md | Adds documentation for factory usage in templates and with @use. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| on.cleanup(() => { | ||
| if (hasTeardown) instance.teardown!(); | ||
| destroy(instance); | ||
| }); | ||
|
|
||
| let updateCache: ReturnType<typeof createCache> | undefined; |
There was a problem hiding this comment.
updateCache created via createCache() is never destroyed on cleanup. If a consumer holds onto the returned proxy past teardown (or if caches retain tracking tags), the cache can keep references alive and may call instance.update() on a destroyed instance via the proxy's get trap. Consider destroying the cache in on.cleanup (and clearing the reference) before/alongside destroying the instance.
| on.cleanup(() => { | |
| if (hasTeardown) instance.teardown!(); | |
| destroy(instance); | |
| }); | |
| let updateCache: ReturnType<typeof createCache> | undefined; | |
| let updateCache: ReturnType<typeof createCache> | undefined; | |
| on.cleanup(() => { | |
| if (updateCache) { | |
| destroy(updateCache); | |
| updateCache = undefined; | |
| } | |
| if (hasTeardown) instance.teardown!(); | |
| destroy(instance); | |
| }); |
| </template> | ||
| ``` | ||
|
|
||
| When `@userId` changes, the resource is re-created with the new variables. |
There was a problem hiding this comment.
The docs say the resource is "re-created" when @userId changes, but the implementation for query/subscription uses an internal createCache to call instance.update() and keep a single instance alive. To avoid misleading readers, consider rewording this to say the query re-executes/updates when the variables change (without implying a new resource instance is created).
| When `@userId` changes, the resource is re-created with the new variables. | |
| When `@userId` changes, the query re-executes and the resource updates with the new variables. |
Summary
queryResource,mutationResource, andsubscriptionResourcefactoryfunctions exported from
glimmer-apollo/resource-factories{{#let}}andin class-based components via the
@usedecorator fromember-resourcesember-resourcesis an optional peer dependency — existing usage ofuseQuery,useMutation, anduseSubscriptionis completely unchangedMotivation
Today, using Apollo with glimmer-apollo requires a backing class for every
component that fetches data. The
ember-resourcesecosystem provides aresource()/resourceFactory()primitive that enables reactive datapatterns directly in templates. This PR bridges glimmer-apollo's existing
Resource classes into that ecosystem without modifying any existing code.
Design
A single
wrapResource()helper wraps any glimmer-apollo Resource subclass(
QueryResource,MutationResource,SubscriptionResource) into anember-resources
resource(). Lifecycle (setup/update/teardown) is derivedfrom the prototype, mirroring
ResourceManager.createHelper. AProxyentangles property access with autotracking, matching the existing
useResourcepattern.Each factory supports two calling conventions:
queryResource(QUERY, opts){{#let}}in templatesqueryResource(() => [QUERY, opts])@usedecoratorAlternative approach
Full integration with ember-resources, where QueryResource, MutationResource, and SubscriptionResource are rewritten as native resource() functions instead of wrapping the existing class-based Resources. This would give native @use support and composability (via ember-resources' use() hook) without needing the Proxy bridge layer. However, it would also change the ergonomics and APIs, maybe not even a good fit for all current glimmer-apollo-resources.
Test plan
pnpm testpasses in test-app (unit + integration)pnpm buildin glimmer-apollo)glimmer-apollo/resource-factoriesresolvesember-resourcesis not installed (existingimports from
glimmer-apollostill work)Cowritten by claude