refactor: migrate everything off the legacy @clevercloud/client esm api - #1817
pdesoyres-cc wants to merge 56 commits into
Conversation
ca345da to
d553ba2
Compare
|
🔎 A preview has been automatically published : https://clever-components-preview.cellar-c2.services.clever-cloud.com/refactor/new-client/index.html. This preview will be deleted once this PR is closed. |
The `install-local:*` scripts packed the dependency in its own directory, then moved the tarball here with a glob. The glob matched the version number, so a leftover tarball from a previous version stayed around and `pnpm add` could pick the wrong one. `pnpm pack --out` writes straight to the target path, so both scripts now produce a file name without a version. Each run overwrites the previous tarball. Refs #1816
`Abortable` swallowed the rejection of an aborted operation by testing for a `DOMException` named `AbortError`. That is the shape the platform rejects with when a raw `fetch()` is aborted. The new `@clevercloud/client` catches that exception and rethrows a `CcRequestError` carrying the `ABORTED` code. An aborted request made through the client therefore escaped the test and was reported as a real error. `isAbortError()` now covers both shapes, and it is exported because callers outside `Abortable` need the same test. Refs #1816
The API token components talk to the auth bridge, not to the main API. That host was reached through `send-to-auth-bridge.js`, which built the OAuth v1 headers by hand. `getCcApiBridgeClient()` returns a `CcApiBridgeClient` built from an `AuthBridgeConfig`, cached by config content like the two existing getters. It dispatches `CcApiErrorEvent` on error, so bridge calls report failures the same way main API calls do. The request config moves into `createClient()`: `isCorsEnabled` replaces the removed `cors` option, and the resource id resolver store is now shared by every client instead of being keyed per config. The resolver maps a resource id to its owner, which does not depend on who asks. Refs #1816
`common.types.d.ts` carried hand-written copies of API payloads, several of them flagged with a `FIXME: this should be provided by the client`. The new `@clevercloud/client` ships those types, so the local copies go away and consumers import them from the package instead. Removed here: `Addon`, `AddonPlan`, `AddonProvider`, `RawAddonProvider`, `Instance`, `PriceSystem`, `Zone`, `Redirection`, `RedirectionNamespace`, `EnvVarRawError`, `EnvVarValidationMode` and `InvoiceStatusType`. The types kept in this file that referred to two of them, `Invoice.status` and `EnvVarEditorStateLoaded.validationMode`, now point at the client types directly. `src/operators.types.d.ts` held `RawOperator`, another copy of an API payload, and is deleted. `DatePart` in the date types had no consumer left. Refs #1816
…w client The invoice and price system helpers went through the legacy `esm/api/v4/billing.js` functions and `sendToApi`. They now send `GetInvoiceCommand`, `GetInvoiceHtmlCommand`, `ListInvoiceCommand` and `GetPriceSystemCommand` through the client returned by `getCcApiClientWithOAuth`. The client returns camelCase invoices, so `formatInvoice` reads `invoiceNumber`, `emittedAt`, `totalTax` and `totalTaxExcluded`. It also became synchronous: the PDF download URL is built by `getUrl(new GetInvoiceUrl(...))`, which signs the URL without a request, where the legacy code had to await the OAuth header before folding it into the query string. The listing now drops invoices with the `LOSS` status. That endpoint applies no status filter, so it hands us invoices written off by accounting. They are frozen and unpayable, and a customer can do nothing with one. `fetchApp` had no caller left. `ONE_DAY` is defined here because the constant lived in the removed `esm/with-cache.js`. Refs #1816
The price system and add-on provider payloads used to reach these formatters in the raw snake_case shape the API returns. The client now normalises them, so the formatters read `runtimes`, `countables`, `pricePlans`, `priceId`, `maxQuantity`, `nameCode`, `computableValue`, `dataQuantityForPrice` and `timeIntervalForPrice`. The local `RawAddonProvider` and `Instance` types are replaced by `AddonProviderFull`, `AddonProviderPlan`, `AddonProviderFeature`, `AddonProviderPlanFeature`, `ProductRuntime` and `ProductRuntimeFlavor` from the client. `formatProductConsumptionIntervals` builds contiguous quantity ranges, so each interval starts where the previous one ends. That only holds because the client orders price plans by `maxQuantity`. The assumption is now written down next to the code that relies on it. Refs #1816
The SSE clients of `@clevercloud/client` changed shape. `LogsSse` is now the `LogsStream` type the
log stream commands return, the listener methods are `onOpen()`, `onLog()` and `onError()` instead
of `on('open')` and `on('error')`, and a fatal HTTP error is matched with
`isCcHttpErrorWithStatus()` rather than an `instanceof HttpError`. Each command also converts the
raw log itself, so `LogsStream` takes a second type parameter for the log shape it receives.
The important change is that `_createStream()` became asynchronous, because the client may need a
network round trip to resolve resource ids. Three things broke because of it.
A stream now finishes being created after the call that asked for it may have been superseded.
`#start()` captures a generation counter, bumped by `stop()` and `complete()`, and drops the
freshly created stream when the counter moved. Such a stream holds no connection yet, so there is
nothing to close.
`pause()` used to be safe because the stream existed and was open. It can now be called while the
stream is still being created, connecting, or retrying, where the client throws. The request is
recorded in `#pausePending` and applied when the stream opens. It has to be deferred by one
microtask: the client fires `open` just before flipping its own state to `open`, so pausing
synchronously in the callback would still see the previous state.
The progression was started on the `connecting` state, which a pending pause moves away from
before the stream opens. `LogsProgress` gained `isStarted()`, and the progression now starts on the
first opening whatever the current state. The waiting timer follows: it detects that no log arrived
for a while, which a paused stream guarantees for a reason it does not report, so `pause()` stops it
and `resume()` starts it back while waiting for the first log.
Refs #1816
The smart component guide documented `sendToApi` next to the two client getters, and told contributors that existing components using it did not need migrating. Both statements are about to stop being true: `sendToApi` is gone and every smart component goes through a `CcApiClient`. The section, its import and the "new versus legacy" framing around the two getters are removed. Refs #1816
The smart component sends `GetAddonCommand`, `ListTagCommand`, `UpdateAddonCommand`, `UpdateTagCommand` and `DeleteAddonCommand` through the client returned by `getCcApiClientWithOAuth`, instead of the legacy `esm/api/v2/addon.js` functions piped into `sendToApi`. Refs #1816
Building the backup list took three requests and a lot of local knowledge. The component fetched the add-on, then the provider specific add-on details, then the backups, and rebuilt each provider's restore command by hand from the connection details it had just read. `ListBackupCommand` returns the backups already carrying their restore and delete commands, and the password those commands need. The per-provider `pg_restore`, `mysql` and `mongorestore` templates are deleted along with the details request. Elasticsearch is the one case left. Telling `es-addon` from `es-addon-old` depends on whether the Kibana service is enabled, which only the Elasticsearch info endpoint knows, so that provider still costs one extra `GetElasticsearchInfoCommand`. `ListBackupCommand` fetches the add-on itself to build the commands, and only after the backups came back. A one second cache entry, shared with `fetchAddon()`, keeps the two from fetching the same add-on twice. It stays short because the same cache holds the backup requests, and a longer entry would start serving a stale list. The failing path was silent: the promise had no `catch`, so the component stayed on its loading state forever. It now logs and moves to the error state. Refs #1816
The four smart variants now send the provider specific commands through the client returned by `getCcApiClientWithOAuth`. Cellar reads `GetCellarCredentialsCommand`, `GetCellarCredentialsPresignedUrlCommand` and `RenewCellarCredentialsCommand`. `cc-addon-credentials.client.js` is deleted. It fetched the add-on only to read its `realId` and provider id, then hand-rolled the add-on provider requests, including the two network group calls that nothing called. The commands take the add-on id and resolve the rest themselves. The provider payload types written by hand here are deleted too: the client ships them, in camelCase. That renames the Elasticsearch service flag from `enabled` to `isEnabled`. Two fixes come with the shapes. The Keycloak variant checked the response for `null`, which the command never returns, and the Pulsar variant built its CLI URL with the `pulsar+ssl://` scheme on the plain, non-TLS port. Refs #1816
The three smart variants send `GetAddonCommand`, `GetZoneCommand` and their provider specific commands through the client returned by `getCcApiClientWithOAuth`. `cc-addon-header.client.js` is deleted, along with the `RawAddon`, `CellarInfo` and `KubeInfo` types written here. It hand-rolled the add-on provider URLs, including the reboot and rebuild calls carrying a `FIXME` asking for the client to provide them. The zone now comes from the add-on's `zone`, which the command returns in place of the old `region`. The Elasticsearch service flag is `isEnabled` instead of `enabled`. Kubernetes keeps its hard-coded Paris zone, since the cluster payload carries no zone, but it moves to a module-level constant next to the reason it exists. Refs #1816
The Cellar and Kubernetes variants were the two left on `sendToApi` with hand-rolled URLs, both carrying a `FIXME` asking for the client to provide them. They now send `GetCellarInfoCommand` and `GetKubernetesClusterCommand`. `CcAddonInfoClient` had nothing else to hold, so only the shared helpers stay in `cc-addon-info.client.js`. Every variant reads the add-on creation date as `createdAt`, the Elasticsearch flags as `isEnabled` and the version list as `availableVersions`. The provider payload types written here are deleted: the client ships them. Two fixes come with the shapes. `GetGrafanaCommand` rejects on 404 where the legacy call resolved with `null`, and the Grafana link is optional, so the call is wrapped in `tolerateNotFound()`. Elasticsearch reports a service as enabled before it has been provisioned, so its application id can be missing, and building a link out of it produced a URL with an `undefined` id. Refs #1816
The smart component sends `ListLinkCommand` and `ListZoneCommand` through the client returned by `getCcApiClientWithOAuth`. `ListLinkCommand` returns every kind of link, not only the applications, so the result is filtered on `link-to-application` and the application is read from `link.application`. The variant logo field is `logoUrl` instead of `logo`. Zones gained an `id` and an `outboundIps`, so the stories are updated. Refs #1816
…ient The smart component fetched the blog RSS feed itself, over a raw request, and parsed the XML with `src/lib/xml-parser.js`. It now sends `ListArticleCommand`, which returns the articles already parsed and caches the feed pages itself. The XML parser existed for this one caller. It is deleted, with its test and the 2000 line feed sample that fed it. An article can have no banner. The parser already handled that, but `ArticleCard.banner` was typed as a required `string`, so the type is corrected to optional. Refs #1816
…odes The controller matched the API error codes as string literals, through the local `isCellarExplorerErrorWithCode()`. The commands declare those codes, so it uses `isCcHttpErrorWithCode()` with `CREATE_CELLAR_BUCKET_ERROR_CODES`, `DELETE_CELLAR_BUCKET_ERROR_CODES` and `GET_CELLAR_BUCKET_ERROR_CODES`. Refs #1816
…odes The controller matched the API error codes as string literals, through the local `isCellarExplorerErrorWithCode()`. It now uses `isCcHttpErrorWithCode()` with `GET_CELLAR_OBJECT_ERROR_CODES`. That one set covers every command targeting a single object: they all declare the same `BUCKET_NOT_FOUND`, and all but the upload declare `OBJECT_NOT_FOUND`. The object listing returns its files under `items` instead of `content`. Refs #1816
`CellarExplorerClient` hand-rolled the nine Cellar URLs and their encoding. It now sends the matching commands through the client returned by `getCcApiClientWithOAuth`. Uploading took two steps: ask for a presigned URL, then `fetch()` the file to it, with its own error handling because the response did not go through `sendToApi`. `UploadCellarObjectCommand` does both. `CellarExplorerError` is deleted. It existed to give the raw response body a code and a context; the client raises `CcHttpError` carrying them, so every `catch` that rewrapped an error goes away too. `CellarAcl` and `CellarGrantee`, written by hand here, come from the client. The smart component no longer resolves the add-on's real id before building the client, since the commands resolve resource ids themselves. That request was the whole loading state, so the component goes straight to the bucket list. Refs #1816
The smart component sends `ListDomainCommand`, `CreateDomainCommand`, `DeleteDomainCommand`, `SetPrimaryDomainCommand`, `UnsetPrimaryDomainCommand` and `GetLoadBalancerInfoCommand` through the client returned by `getCcApiClientWithOAuth`. `parseDomain()` and friends move to `utils/domain-utils.js`, where `parseDomain()` returns `pathPrefix` instead of `pathname`. `ListDomainCommand` returns each domain with its `isPrimary` flag, so the separate primary domain request goes away, and with it the 404 it raised on an application with no primary domain. `convertApiError()` is deleted. It carried two `FIXME` asking the API for real error ids, and matched English error messages with `includes()` to tell an invalid domain from a taken one. The command declares `CREATE_DOMAIN_ERROR_CODES`, and setting the primary domain matches `SET_PRIMARY_DOMAIN_ERROR_CODES.NOT_FOUND` instead of the raw numeric id `3004`. Adding or removing a domain now refetches the list instead of patching it locally. The local patch had to invent an id for the new domain, and it could not know that the API had just moved the primary flag. The story for an application with no primary domain is added, since that case is now reachable without an error. Refs #1816
The smart component sends the profile email commands through the client returned by `getCcApiClientWithOAuth`. Three of the legacy calls it replaces were still prefixed `todo_`. `ListProfileEmailAddressCommand` returns the primary and the secondary addresses together, so the separate request for the user's own profile goes away. It also reports a verification flag per secondary address, where the component used to hard-code `verified: true` on all of them. Adding an address matches `CREATE_PROFILE_EMAIL_ADDRESS_ERROR_CODES` instead of the raw numeric ids `550`, `101` and `1004`. Making a secondary address primary has its own `SetProfilePrimaryEmailAddressCommand`, instead of re-posting the address with a `make_primary` flag. Refs #1816
The smart component sends `GetGrafanaCommand`, `EnableGrafanaCommand`, `DisableGrafanaCommand` and `ResetGrafanaCommand` through the client returned by `getCcApiClientWithOAuth`. An owner with no Grafana organisation is a 404, which the component used to recognise by testing the status and then matching the English error message. It now wraps the call in `tolerateNotFound()`. Enabling returns the created organisation, so the component builds the link from it instead of refetching. Both places that build that link share `buildGrafanaLink()`. Refs #1816
`validateName()` moved from `esm/utils/env-vars.js` to `utils/environment-utils.js`, and `EnvVarValidationMode` now comes from the client instead of `common.types.js`. The documentation link pointing at the old source file is updated. Refs #1816
The parsing helpers moved from `esm/utils/env-vars.js` to `utils/environment-utils.js`. The parsing error type is `EnvVarParsingError`, from the client, instead of the local `EnvVarRawError`. Refs #1816
The parsing helpers moved from `esm/utils/env-vars.js` to `utils/environment-utils.js`. The parsing error type is `EnvVarParsingError`, from the client, instead of the local `EnvVarRawError`. Refs #1816
The controller opens its stream with `StreamApplicationRuntimeLogCommand`, and the instance and deployment requests go through `GetDeploymentCommand`, `GetDeploymentCommandLegacy`, `ListApplicationInstanceCommand` and `GetApplicationInstanceCommand`. `GetDeploymentCommandLegacy` normalises the v2 deployment payload, so the local mapping from `WIP`, `OK` and `FAIL` to the v4 states is deleted, along with the `uuid` to `id` rename. Instances report `createdAt` and `deletedAt` instead of `creationDate` and `deletionDate`. The deployment states worth caching, the ones with an end date, and the ones a fetch can skip are all the same three, so they share `isFinalDeploymentState()`. Two lists had drifted: the fetch filter listed only `QUEUED` and `WORK_IN_PROGRESS`, missing `TASK_IN_PROGRESS`. 404 is matched with `isCcHttpErrorWithStatus()`. Fetching an unknown instance used to fall through its `catch` and convert `undefined` on any other error, so it now rethrows. Refs #1816
…from the client `DeploymentState` and `InstanceState` were listed by hand here. They come from the client instead. `InstanceState` excludes `GHOST`, which never reaches this component because a ghost instance is modelled by `GhostInstance`. The client's list has a `TASK_IN_PROGRESS` deployment state the local one missed, so a deployment in that state was rendered as finished. It joins `DEPLOYMENT_WIP_STATES`, and the deleted instance icon matches it too, through `DEPLOYMENT_ONGOING_STATES`. That second list leaves `QUEUED` out: nothing has started yet, so a queued deployment cannot have deleted an instance. Refs #1816
…ork group `GetNetworkGroupCommand` rejects when the network group is missing, it never resolves with `null`, so the branch testing for it was dead. Refs #1816
A peer endpoint reports `networkGroupTerm` and `networkGroupIp` instead of `ngTerm` and `ngIp`. Whether an add-on can join a network group was decided here by comparing its plan slug to `dev`. The client exposes that rule as `isNetworkGroupAddonCandidate()`, which the member list already uses. `ONE_DAY` is defined here because the constant lived in the removed `esm/with-cache.js`. Refs #1816
…up shapes A peer endpoint reports `networkGroupTerm` and `networkGroupIp` instead of `ngTerm` and `ngIp`, and an application variant reports `logoUrl` instead of `logo`. The member logo requests are wrapped in `tolerateNotFound()`. A deleted application or add-on answers with a 404, which the commands reject on where the legacy calls resolved with `null`, and that case is expected: the member is then reported as deleted. The API also lists the network group's load balancers as members. They are registered by the load balancer itself and this component offers no way to manage one, so they are filtered out. Refs #1816
Three components spread a map of every OAuth consumer right set to `false` under the rights the API returned, so the ones it leaves out are still represented. Each of them wrote that map by hand, which means three lists to keep in sync with the API. `DISABLED_RIGHTS` builds it from `OAUTH_CONSUMER_RIGHTS`, the list the client declares. Refs #1816
The two smart variants send `CreateOauthConsumerCommand`, `GetOauthConsumerCommand`, `UpdateOauthConsumerCommand` and `DeleteOauthConsumerCommand` through the client returned by `getCcApiClientWithOAuth`. The commands take and return the rights in camelCase, so the conversions in both directions are gone. The three lists of right names written here are replaced by `ACCESS_RIGHTS`, `MANAGE_RIGHTS` and `GRANTABLE_RIGHTS` from the client, and the all-false map by `DISABLED_RIGHTS`. Filtering out `almighty` by hand goes with them: it is not in `GRANTABLE_RIGHTS`. Refs #1816
The smart component sends `GetOauthConsumerCommand` and `GetOauthConsumerSecretCommand` through the client returned by `getCcApiClientWithOAuth`. The commands return the rights in camelCase, so the conversion is gone, and the all-false map is `DISABLED_RIGHTS`. The two lists of right names written here are replaced by `ACCESS_RIGHTS` and `MANAGE_RIGHTS` from the client. Refs #1816
The smart component sends `ListOrganisationMemberCommand`, `AddOrganisationMemberCommand`, `UpdateOrganisationMemberCommand`, `RemoveOrganisationMemberCommand` and `GetProfileCommand` through the client returned by `getCcApiClientWithOAuth`. The member list arrives flat and in camelCase, so the mapping no longer digs into a nested `member` and reads `jobTitle`, `emailAddress` and `preferredMfa`. Errors are matched with the codes the commands declare, instead of the raw numeric ids `6451`, `6452` and `6501`. The rate limit was recognised by comparing the error message to the English sentence the API returns, and now goes through `isRateLimitError()`. A `@ts-expect-error` about a required `invitationKey` is gone: the command does not ask for one. Refs #1816
The smart component sends `GetProfileCommand`, `ListPersonalSshKeyCommand`, `CreatePersonalSshKeyCommand`, `DeletePersonalSshKeyCommand` and `ListGithubSshKeyCommand` through the client returned by `getCcApiClientWithOAuth`. Three of the legacy calls it replaces were still prefixed `todo_`. The profile reports `isLinkedToGitHub`, so the component no longer looks for `github` in a list of linked OAuth applications. The calls move into an `Api` class, as in the other smart components, instead of passing `apiConfig` and the signal to every free function. Refs #1816
The story fixtures were copies of the raw API payloads, in snake_case, typed against the local copies of those shapes. Both are gone, so the fixtures now match what the commands return. The price system fixtures are renamed from `rawPriceSystem*` to `priceSystem*`: they are no longer raw. Zones gain an `id` and an `outboundIps`, and runtime flavors a `cpuFactor` and a `memFactor`. Refs #1816
The smart component sends `ListZoneCommand` through the client returned by `getCcApiClientWithOAuth`, and `Zone` comes from the client instead of `common.types.js`. `ONE_DAY` is defined here because the constant lived in the removed `esm/with-cache.js`. Refs #1816
Both smart variants send `ListProductAddonCommand` and `ListProductRuntimeCommand` through the client returned by `getCcApiClientWithOAuth`. The products are typed by the client, as `ProductAddon` and `ProductRuntime`, so the local `RawAddonProvider` and `Instance` go away. The add-on listing asks for `withVersions: false`. Version information comes from another endpoint and the pricing page does not show it. Two `@ts-expect-error` about parameters required by the types on public endpoints are gone. One remains, on the hard-coded runner products, whose shape does not strictly match `ProductRuntime`. The stories follow the renamed price system fixture. Refs #1816
The price system fixtures are named `priceSystemEuro` and `priceSystemDollars`. The pricing page sandbox follows the same rename, and takes `PriceSystem` from the client. Refs #1816
`Zone` comes from `@clevercloud/client` instead of `common.types.js`. It carries an `id` and an `outboundIps`, so the skeleton zone and the stories are updated. Refs #1816
The smart component sends `ListTcpRedirectionNamespaceCommand`, `ListTcpRedirectionCommand`, `CreateTcpRedirectionCommand` and `DeleteTcpRedirectionCommand` through the client returned by `getCcApiClientWithOAuth`. The commands type the payloads, so the two local `@typedef` describing them go away. The calls move into an `Api` class, as in the other smart components, instead of passing `apiConfig` and the signal to every free function. Refs #1816
The Grafana organisation is fetched with `GetGrafanaCommand` through the client returned by `getCcApiClientWithOAuth`. An owner with no Grafana organisation is a 404, which the component used to reach through its `catch` along with every other failure. The call is wrapped in `tolerateNotFound()`, so that expected case returns `null` and falls back to the Console Grafana page, while a real error still rejects. Refs #1816
`ERROR_TYPES` moved from `esm/utils/payment.js` to `utils/payment-utils.js`. Refs #1816
Creating the token goes through `CreateApiTokenCommand` on the auth bridge client returned by `getCcApiBridgeClient()`, and the user profile through `GetProfileCommand` on the main client. The hand-rolled auth bridge request is gone. The profile reports `emailAddress` and `preferredMfa`, and the command takes the expiration as `expiresAt`. The error code is read with `isCcHttpError()` instead of digging into `responseBody`. Refs #1816
Listing and revoking tokens go through `ListApiTokenCommand` and `DeleteApiTokenCommand` on the auth bridge client returned by `getCcApiBridgeClient()`. The profile and the password reset go through `GetProfileCommand` and `RequestAuthPasswordResetCommand` on the main client. The four hand-rolled requests are gone, and with them the `partner_id` and `drop_tokens` snake_case body. The commands type the payloads, so the local `RawApiToken` is deleted. Tokens report `createdAt` and `expiresAt`, and the profile `emailAddress`. Refs #1816
Reading and updating the token go through `GetApiTokenCommand` and `UpdateApiTokenCommand` on the auth bridge client returned by `getCcApiBridgeClient()`. The two hand-rolled requests are gone. Refs #1816
The smart component sends `ListOauthTokenCommand` and `DeleteOauthTokenCommand` through the client returned by `getCcApiClientWithOAuth`. Both legacy calls it replaces were still prefixed `todo_`. Tokens report `createdAt`, `expiresAt` and `lastUsedAt` instead of `creationDate`, `expirationDate` and `lastUtilisation`, and the command types them, so the local `RawTokenData` is no longer needed here. Refs #1816
The smart component sends `ListOauthTokenCommand` and `DeleteOauthTokenCommand` through the client returned by `getCcApiClientWithOAuth`. Both legacy calls it replaces were still prefixed `todo_`. Tokens report `createdAt`, `expiresAt` and `lastUsedAt` instead of `creationDate`, `expirationDate` and `lastUtilisation`. The command types them, so the local `RawTokenData` is deleted. Refs #1816
`sendToApi` and `sendToAuthBridge` prefixed the URL, added the OAuth v1 header, wrapped the request in a cache and sent it. Every smart component now goes through a `CcApiClient` or a `CcApiBridgeClient`, which do all of that themselves, so neither helper has a caller left. `send-to-api.types.js` and `send-to-api.events.js` stay: the configuration types and the error event are still used. Refs #1816
The plugin resolved modules with node10 resolution, which ignores the exports field. @clevercloud/client only exposes its subpaths that way, so every @import pointing at it failed and the CEM build exited with code 1. Resolution alone was not enough: type files reach those types through bare imports, which the plugin skipped, so their source was never read and no interface was inlined. Following bare imports also means hitting side effect imports, which have no import clause.
d553ba2 to
b792eef
Compare
🧐 Visual tests report for PR #1817The latest visual tests report is available. Please review the results.
3 components impacted
This comment was generated automatically by the Visual tests workflow. |
florian-sanders-cc
left a comment
There was a problem hiding this comment.
Gg @pdesoyres-cc I really like the new client so I reallyyyyy like this PR 🙌
I only have one small issue with the way errors are caught in cc-token-api-creation-form but the rest are just nitpicks!
| export function fetchApp({ apiConfig, signal, ownerId, appId }) { | ||
| return getApp({ id: ownerId, appId }).then(sendToApi({ apiConfig, signal })); | ||
| } | ||
| const ONE_DAY = 1000 * 60 * 60 * 24; |
There was a problem hiding this comment.
nitpick: we still use ONE_DAY in quite a few cases, can't we export it from here rather than redefining it everywhere?
| type: rawInvoice.type || 'INVOICE', | ||
| number: rawInvoice.invoiceNumber, | ||
| emissionDate: rawInvoice.emittedAt, | ||
| type: /** @type {Invoice['type']} */ (rawInvoice.kind) || 'INVOICE', |
There was a problem hiding this comment.
question: why do we need the cast + the fallback to 'INVOICE'? (the type seems to point to non optional + it already fits Invoice['type']) 🤔
| // The "es-addon" / "es-addon-old" distinction (and its password) can only be resolved through | ||
| // the Elasticsearch specific info endpoint (it depends on whether the Kibana service is enabled). | ||
| if (addon.provider.id === 'es-addon') { | ||
| const esInfo = await this.fetchElasticsearchInfo({ addonId }); | ||
| const kibana = esInfo.services.find((service) => service.name === 'kibana'); | ||
| providerId = kibana != null && kibana.isEnabled ? 'es-addon' : 'es-addon-old'; | ||
| passwordForCommand = esInfo.config.password; | ||
| } |
There was a problem hiding this comment.
question: should this logic move to the client later on? I imagine we have similar needs in the CLI?
| if (isCcHttpErrorWithCode(error, CREATE_CELLAR_BUCKET_ERROR_CODES.BUCKET_ALREADY_EXISTS)) { | ||
| this.#updateCreateForm({ type: 'idle', error: 'bucket-already-exists' }); | ||
| } else if (isCellarExplorerErrorWithCode(error, 'clever.file-explorer-proxy.cellar.bucket-name-invalid')) { |
There was a problem hiding this comment.
question: I tried to create a bucket with a name that already exists, didn't trigger the corresponding error message (got the typical error notice instead).
Same happens in prod so it's not a new subject, this is the error payload I got:
{
"apiRequestId": "request_7db9792d-1547-47a3-a413-240f564c7de4",
"code": "clever.cellar.bucket-already-exists",
"context": { "kind": "bucket", "name": "bob-empty-bucket-1", "type": "Resource" },
"error": "Bucket 'bob-empty-bucket-1' already exists"
}
| if (apiErrorId === 101) { | ||
| if (apiErrorCode === CREATE_PROFILE_EMAIL_ADDRESS_ERROR_CODES.ALREADY_DEFINED) { | ||
| return 'already-defined'; | ||
| } |
There was a problem hiding this comment.
question: just like with cellar, this doesn't seem to work, I get a generic toast instead of the specific error message
API Response:
{ "id": 4001, "message": "This email address already belongs to you", "type": "error" }
| * @returns {peer is KnownNetworkGroupPeer} | ||
| */ | ||
| function isKnownNetworkGroupPeer(peer) { | ||
| return isKnown(peer) && isKnown(peer.endpoint); |
There was a problem hiding this comment.
question: I guess I missed this change in the client 🙈 Could you tell me more about why we have to filter out some peers? (mostly for my information)
| avatar: member.avatar, | ||
| name: member.name, | ||
| jobTitle: member.jobTitle, | ||
| role: /** @type {OrgaMemberRole} */ (member.role), |
There was a problem hiding this comment.
nitpick: this cast could cause issues if roles evolve at some point (like they did recently).
We're not supposed to get role: 'NONE' here, but casting basically turns TS off for this line: if a role gets added to the client, it will go through the cast silently and the card will receive a role it doesn't handle.
It could be more robust to narrow instead, like skip NONE members in a flatMap (or throw if we'd rather know about it). This way, TS would flag any mismatch between the client roles and OrgaMemberRole.
| 'responseBody' in error && typeof error.responseBody === 'object' && 'code' in error.responseBody | ||
| ? error.responseBody.code | ||
| : null; | ||
| const errorCode = isCcHttpError(error) ? error.code : null; |
There was a problem hiding this comment.
fix: in prod, if I enter a wrong password, I get the error message below the input field and I can try again.
With this branch, I see the error below the input but I get disconnected just after that. This makes me think the error is not swallowed properly? (didn't dig further)
Closes #1816
Context
clever-componentsimported two API clients from@clevercloud/client. The command client lives undercc-api-commands/,cc-api-bridge-commands/,redis-http-commands/andutils/. The legacy service and function API lives underesm/.53 files still imported the legacy one.
Problem
clever-client.jscannot deprecate itsesm/API whileclever-componentsremains one of its largest consumers. That blocks the GA release of the command client.The mixed state also costs us here. Two clients mean two error models, two authentication paths and two caching strategies.
src/lib/send-to-api.jshand-rolled URL prefixing, OAuth signing, caching and timeouts, all of which the command client already does.Proposal
Every remaining
esm/import moves to the command client. The legacy plumbing is then deleted, since nothing calls it any more.The branch now leaves zero
esm/imports insrc/,test/anddocs/.What lands, by area:
sendToApinow build a client withgetCcApiClient()orgetCcApiClientWithOAuth()and send commands.send-to-api.jsandsend-to-auth-bridge.jsare removed.api-helpers.jssends invoice and price system commands.cc-api-client.jsgainsgetCcApiBridgeClient(), so the API token components reach the auth bridge through a real client instead of hand-built OAuth v1 headers.common.types.d.tsdropped its hand-written copies of API payloads, several of which carried aFIXMEasking for exactly that.src/operators.types.d.tsis deleted. Consumers import those shapes from the client package.xml-parser.jsand its test are gone, along with the two.client.jsfiles that wrapped Cellar and Kubernetes calls.Design decisions
fetchApp()disappeared fromapi-helpers.jsrather than being ported. Its two callers sendGetApplicationCommanddirectly, which is one less indirection for one call.isAbortError()insrc/lib/abortable.jsaccepts two error shapes. The platform rejects with aDOMExceptionnamedAbortError, and the client rethrows aCcRequestErrorcarrying theABORTEDcode. Both mean the same thing, so both are treated as an abort.The
install-local:*scripts used a glob to move a packed tarball. The glob matched the version number, so a leftover tarball could win. Both scripts now pack straight to a fixed file name withpnpm pack --out.How to review
Read the history, not the global diff. The 56 commits are ordered and each one carries a body explaining its own change. The global diff is mostly fixture churn.
Two commits deserve real attention:
98947ebf refactor(lib): adapt the logs stream to the new clientis the only place where behaviour changed rather than the call shape._createStream()became asynchronous, which broke stream cancellation,pause()and the progression start. The commit body walks through the three fixes, andtest/logs/logs-stream.test.jscovers them.7d4ef4a7 feat(lib): add a cached auth bridge clientintroducesgetCcApiBridgeClient()and moves the request config intocreateClient().The rest of the commits repeat the same pattern, one component at a time. Reading two or three of them is enough to know what the others do.
How to test
The migration is meant to be invisible from the outside. Every smart component should behave as it did before.
pnpm run testandpnpm run typecheck.pnpm run storybook:devand exercise a few smart components against a real API token.