Skip to content

Commit 4b411e7

Browse files
authored
Merge pull request #19883 from mozilla/fxa-12881
chore(docs): Add ADR for removing graphql from fxa-settings
2 parents d0f1a4b + 25351b2 commit 4b411e7

1 file changed

Lines changed: 125 additions & 0 deletions

File tree

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
# Remove GraphQL from fxa-settings
2+
3+
- Status: proposed
4+
- Deciders: Vijay Budhram, Lauren Zugai, Dan Schomburg, Barry Chen, Valerie Pomerleau, Amri Toufali, Reino Muhl
5+
- Date: 2026-01-14
6+
7+
This ADR looks at fully removing GraphQL and Apollo from fxa-settings, building on [ADR-0044](0044-use-rest-over-gql-when-convenient.md) which established a preference for auth-client over GraphQL.
8+
9+
## Context and Problem Statement
10+
11+
In [ADR-0016](0016-use-graphql-and-apollo-for-settings-redesign.md) (2020), FxA adopted GraphQL and Apollo for the Settings Redesign project. The chosen approach was "Option B" - layering GraphQL on top of existing REST architecture via `fxa-graphql-api` as a proxy to `fxa-auth-server`.
12+
13+
Four years later, [ADR-0044](0044-use-rest-over-gql-when-convenient.md) (2024) acknowledged that the GraphQL experience had been "a mixed bag" and established a preference for using `auth-client` for new network requests. However, ADR-0044 stopped short of committing to remove GraphQL entirely, citing concerns about level-of-effort and potential future framework changes (NextJS).
14+
15+
This ADR evaluates whether to complete that migration by fully removing GraphQL from fxa-settings, or to maintain the current mixed approach.
16+
17+
**Key question**: Should we fully remove GraphQL and Apollo from fxa-settings?
18+
19+
## Decision Drivers
20+
21+
- **Operational complexity**: Running and maintaining `fxa-graphql-api` as a separate service
22+
- **Infrastructure overhead**: Currently requires 4 additional Fastly sites (prod/stage for GraphQL and internal auth endpoints), generating monitoring noise despite lower traffic volumes
23+
- **Security surface area**: GraphQL exposes a flexible query language that requires allowlists to restrict operations; removing it eliminates this attack vector and the need for allowlist generation/maintenance
24+
- **Rate limiting**: Setting up firewall-level rate limits for GraphQL is a pain since everything goes through one endpoint
25+
- **Error monitoring**: With GraphQL, we can't easily get error rates or request counts per operation in Sentry or our dashboards
26+
- **Developer experience**: Cognitive load of maintaining two data-fetching paradigms
27+
- **Testing complexity**: Apollo mocking vs direct auth-client mocking
28+
- **State management**: Apollo Cache reactivity vs localStorage with `useSyncExternalStore`
29+
- **Performance**: Direct auth-server calls vs GraphQL proxy overhead
30+
- **Future flexibility**: Positioning for potential NextJS migration
31+
32+
## Considered Options
33+
34+
- **Option A**: Maintain current mixed GraphQL/auth-client approach (status quo from ADR-0044)
35+
- **Option B**: Fully remove GraphQL from fxa-settings, use auth-client + localStorage with useSyncExternalStore
36+
- **Option C**: Go "all-in" on GraphQL by refactoring to use libs directly in graphql-api
37+
38+
## Decision Outcome
39+
40+
**Chosen option: Option B** - Fully remove GraphQL from fxa-settings.
41+
42+
- We're maintaining 4 extra Fastly sites, generating allowlists, and running a whole separate service for GraphQL. That's a lot of overhead for what we're getting out of it.
43+
- Having two ways to fetch data (GraphQL and auth-client) is confusing.
44+
- We never really used Apollo Cache the way it was meant to be used anyway, so we're not losing much there.
45+
46+
### Positive Consequences
47+
48+
- **Reduced operational complexity**: No need to run/maintain `fxa-graphql-api` for settings
49+
- **Infrastructure simplification**: Eliminates 4 Fastly sites (prod/stage for GraphQL and internal auth endpoints) that currently generate monitoring noise and operational overhead despite lower traffic volumes
50+
- **Reduced attack surface**: Removes GraphQL query language exposure and eliminates the need for query allowlist generation/maintenance; attackers can no longer craft arbitrary GraphQL queries
51+
- **Simpler testing**: Direct auth-client mocking instead of Apollo MockedProvider
52+
- **Clearer data flow**: Components → auth-client → auth-server (no GraphQL proxy layer)
53+
- **Cleaner architecture**: We stop proxying calls through graphql-api just to reach auth-server
54+
- **Native React state management**: Uses React 18's `useSyncExternalStore` for localStorage reactivity instead of Apollo Cache
55+
- **Smaller bundle size**: Removing `@apollo/client` dependency (~100KB+ gzipped)
56+
- **Faster development**: No need to create GraphQL resolvers, mutations, and types for new features
57+
- **Better error handling**: Direct REST errors vs GraphQL error wrapping
58+
- **Better monitoring**: Sentry and our dashboards work better with REST endpoints since we get clear per-endpoint error rates and request counts
59+
- **Positioned for NextJS**: NextJS has its own opinions about data fetching (Server Components, server actions, etc.) that don't mesh well with Apollo's client-side cache. If we migrate to NextJS with Apollo still in place, we'd have to rip it out then anyway. Better to do it now when we can take our time, rather than during a framework migration when we're already juggling a lot.
60+
61+
### Negative Consequences
62+
63+
- **Migration effort**: Requires completing migration of remaining GraphQL usages
64+
- **Custom state synchronization**: Must use `useSyncExternalStore` with custom events for localStorage reactivity (replacing Apollo Cache's built-in reactivity). Note: TanStack Query could help here if we want caching and automatic refetching, but it's not required for our current needs.
65+
- **Lost GQL benefits**: Lose declarative data requirements, GraphQL playground, and schema documentation (though in practice, nobody on the team actively uses the playground)
66+
- **Requires server changes**: Auth-server endpoints must return all needed data (consolidated endpoints)
67+
- **Testing patterns change**: Teams must learn new mocking patterns (though we already have auth-client mocking patterns in place from existing code)
68+
69+
## Pros and Cons of the Options
70+
71+
### Option A: Maintain current mixed GraphQL/auth-client approach
72+
73+
This is the status quo from ADR-0044 - prefer auth-client but keep GraphQL where already set up.
74+
75+
- Good, because no additional migration work required
76+
- Good, because preserves existing GraphQL investments
77+
- Good, because maintains flexibility to go either direction
78+
- Bad, because developers must understand two data-fetching paradigms
79+
- Bad, because Apollo Cache manual updates are error-prone and cause bugs
80+
- Bad, because tests require both Apollo mocks and auth-client mocks
81+
- Bad, because `fxa-graphql-api` service must continue running
82+
- Bad, because new features face decision fatigue (use GQL or auth-client?)
83+
84+
### Option B: Fully remove GraphQL from fxa-settings
85+
86+
Complete the migration to auth-client with localStorage for persistence and `useSyncExternalStore` for reactive state management.
87+
88+
**Why `useSyncExternalStore`?** One thing Apollo gives us is automatic re-renders when cached data changes. When we remove Apollo, we lose that. We still need components to update when account data changes (e.g., user updates their display name and it should show up everywhere). React 18's `useSyncExternalStore` lets us do this with localStorage: components subscribe to changes, and when we update the stored account data, all subscribed components re-render. It's built into React, so no extra libraries needed.
89+
90+
- Good, because single data-fetching pattern reduces cognitive load
91+
- Good, because testing is simpler with direct auth-client mocks
92+
- Good, because eliminates GraphQL service dependency for settings
93+
- Good, because consolidated `/account` endpoint reduces API calls (9 → 3)
94+
- Good, because positions codebase well for NextJS migration (NextJS prefers server-side data fetching which doesn't play nice with Apollo's client-side cache; removing it now means one less thing to deal with during migration)
95+
- Good, because reduces attack surface by eliminating GraphQL query language exposure and allowlist requirements
96+
- Good, because eliminates 4 Fastly sites and associated operational/monitoring overhead
97+
- Good, because smaller bundle size without Apollo client
98+
- Bad, because requires completing migration work
99+
- Bad, because replaces Apollo Cache reactivity with custom `useSyncExternalStore` + localStorage pattern
100+
- Bad, because loses GraphQL playground and self-documenting schemas
101+
- Bad, because requires understanding `useSyncExternalStore` pattern for localStorage reactivity
102+
103+
### Option C: Go "all-in" on GraphQL
104+
105+
Refactor to use libs directly in graphql-api (deprecate auth-client), as described in FXA-8633.
106+
107+
- Good, because eliminates mixed paradigm by going fully GraphQL
108+
- Good, because could achieve better type safety with GraphQL codegen (though currently we manually declare model shapes on both client and server, so we don't get this benefit today)
109+
- Good, because eliminates Apollo Cache manual update issues
110+
- Good, because maintains GraphQL tooling benefits
111+
- Bad, because requires significant backend refactoring
112+
- Bad, because may conflict with NextJS migration plans
113+
- Bad, because increases GraphQL service complexity
114+
- Bad, because moving away from GQL later would be very expensive
115+
- Bad, because requires migrating auth-client usage back to GraphQL
116+
117+
## Links
118+
119+
- Previous ADRs:
120+
- [ADR-0016: Use GraphQL and Apollo for Settings Redesign](0016-use-graphql-and-apollo-for-settings-redesign.md)
121+
- [ADR-0044: Use REST over GQL when convenient](0044-use-rest-over-gql-when-convenient.md)
122+
- [ADR-0039: Use container component pattern](0039-use-react-container-component-abstraction.md)
123+
- Related tickets:
124+
- [FXA-8633: Epic for moving away from auth-client in graphql-api](https://mozilla-hub.atlassian.net/browse/FXA-8633)
125+
- [FXA-10861: Create auth-client wrapper](https://mozilla-hub.atlassian.net/browse/FXA-10861)

0 commit comments

Comments
 (0)