feat(docs): add registration page and update sidebar and account documentation page - #2689
Maciej D (mdanilowicz) wants to merge 1 commit into
Conversation
Patryk Tomczyk (patzick)
left a comment
There was a problem hiding this comment.
🔍 Code Review
Docs-only PR. Structure, frontmatter and sidebar match the existing login/wishlist recipes, all SchemaTypeTooltip type keys resolve against storeApiSchema.json, all Related Links targets exist, and the useUser().register() behaviour described (storefrontUrl injection, active && !doubleOptInRegistration gate, awaited refreshSessionContext(), no refreshCart(), isLoggedIn formula, rethrow on context failure) is accurate against packages/composables.
Six findings, all inline. One is a wrong technical claim (window.location.origin during SSR), the rest are accuracy/completeness gaps in a page whose value is being exactly right.
Note on CI: every check on this PR is green, but code-check.yml has paths-ignore: ["apps/**", "**.md"], so Build, Typecheck, lint and tests did not run on this diff — the passing lint check belongs to the Check PR title workflow. The new 493-line Vue-in-markdown page has therefore never been compiled by CI; please run the docs build locally before marking ready.
| - The generated `billingAddress` type is `Schemas["CustomerAddress"]`, which requires `id` and `customerId`. Send placeholder values as both starter templates do, because you cannot know the ids of an address that does not exist yet. | ||
| - A registration with double opt-in enabled resolves successfully while leaving `isLoggedIn` false. Branch on the returned `doubleOptInRegistration` flag instead of assuming a session exists after the promise resolves. | ||
| - `refreshSessionContext()` rethrows after logging, so a failing `readContext get /context` rejects the `register()` promise even though the customer was already created. A retry then hits `VIOLATION::CUSTOMER_EMAIL_NOT_UNIQUE`. | ||
| - `getStorefrontUrl()` falls back to `window.location.origin`, which is empty during server-side rendering. Submit the form from the client, and set `devStorefrontUrl` when the Shopware domain differs from the origin your app runs on. An origin that is not a configured sales channel domain comes back as a constraint violation pointing at `/storefrontUrl`, which no field in your form owns. |
There was a problem hiding this comment.
major — wrong SSR failure mode. getStorefrontUrl() is return devStorefrontUrl ?? window.location.origin ?? ""; (useInternationalization.ts:92-94). On the server window is not defined, so this throws a ReferenceError — it does not return an empty string. The advice that follows is right; only the stated mechanism is wrong.
Suggested: "…falls back to window.location.origin, which is not available during server-side rendering, so calling register() on the server throws."
| } | ||
| ); | ||
|
|
||
| // The generated body is a union, so accountType is a literal and vatIds can be |
There was a problem hiding this comment.
minor — the payload type is not a union. Omit<T, K> is Pick<T, Exclude<keyof T, K>>, which flattens Body & (private | business) into a single object type. On RegisterPayload the fields end up as accountType: "private" | "business", company: string | null | undefined, vatIds: [string, ...string[]] | null | undefined, so form.accountType = "business" and form.vatIds = [v] compile directly (verified with tsc 5.9 --strict).
Two consequences worth stating instead:
- the computed models are a convenience for
v-model+ nullable fields, not a type requirement; - the discrimination the Types section describes is lost on
RegisterPayload, so nothing stops you sendingaccountType: "business"without acompany— that is enforced by the Store API, not the compiler.
| | Step | Code | Store API | Type | | ||
| | -------------------------------- | ---------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------- | | ||
| | Load salutation options | `fetchSalutations()` | `POST /salutation` | <SchemaTypeTooltip type-key='operations["readSalutation post /salutation"]["response"]' /> | | ||
| | Load countries and their states | `fetchCountries()` | `POST /country` | <SchemaTypeTooltip type-key='operations["readCountry post /country"]["response"]' /> | |
There was a problem hiding this comment.
minor — the route depends on cacheableReads. Both composables switch to the GET variants when cacheableReads is set on the Shopware context:
useSalutations.ts:34-36→readSalutationGet get /salutationuseCountries.ts:45-51→readCountryGet get /country
So POST /salutation / POST /country in this table (and the two operations entries in the frontmatter) only describe the non-cacheable path. Worth listing both, as the rest of the page is precise about which route runs.
|
|
||
| ## Edge Cases | ||
|
|
||
| - The generated `billingAddress` type is `Schemas["CustomerAddress"]`, which requires `id` and `customerId`. Send placeholder values as both starter templates do, because you cannot know the ids of an address that does not exist yet. |
There was a problem hiding this comment.
minor — only one template does this. templates/vue-starter-template/app/components/account/RegistrationForm.vue:51-53 sends id: "" / customerId: "", but templates/vue-demo-store/app/components/account/RegistrationForm.vue:43-50 omits both (its state is untyped) and registers fine. So the placeholders are a way to satisfy the generated CustomerAddress type, not something the Store API needs. Please reword to say that, and drop "both starter templates".
| lastName: "", | ||
| email: "", | ||
| password: "", | ||
| company: "", |
There was a problem hiding this comment.
minor — company placement diverges from both shipped forms. The example puts company at the top level of the body (which matches the business branch of the generated type), while vue-starter-template binds state.billingAddress.company and vue-demo-store binds state.billingAddress.company too. These are two different fields (customer vs. address). Since the recipe's job is to explain what Shopware does with the body, one sentence on which one the register route actually consumes — and why the templates use the address field — would prevent readers from getting a different result than the templates they copy from.
| ## Composables | ||
|
|
||
| - `useUser`: exposes `register`, which returns the created `Schemas["Customer"]`, plus `user`, `isLoggedIn`, `isCustomerSession`, `isGuestSession`, and `refreshUser` for the state you render afterwards. | ||
| - `useSalutations`: exposes `getSalutations` and `fetchSalutations`. It fetches the list on mount when nothing has been provided yet and shares it through the `swSalutations` injection, so several forms on one page issue one request. |
There was a problem hiding this comment.
nit — the option lists are client-only. Both useSalutations and useCountries load via onMounted (useSalutations.ts:52, useCountries.ts:81), so getSalutations and getCountriesOptions are empty during SSR and the selects render with only the placeholder option on the server. Edge Cases covers SSR for storefrontUrl but not for these; a line here (or an explicit fetchSalutations()/fetchCountries() call in the example) would close the gap.
Description
Type of change
ToDo's
Screenshots (if applicable)
Additional context