From 3ecfb121859d8445f745232e8501340416fffe5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Fri, 29 Nov 2024 13:42:23 +0100 Subject: [PATCH] move to a new ADR MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- ...-use-fetch.md => adr013-use-node-fetch.md} | 24 ++++-- .../adr014-use-fetch.md | 78 +++++++++++++++++++ mkdocs.yml | 3 +- 3 files changed, 96 insertions(+), 9 deletions(-) rename docs/architecture-decisions/{adr013-use-fetch.md => adr013-use-node-fetch.md} (72%) create mode 100644 docs/architecture-decisions/adr014-use-fetch.md diff --git a/docs/architecture-decisions/adr013-use-fetch.md b/docs/architecture-decisions/adr013-use-node-fetch.md similarity index 72% rename from docs/architecture-decisions/adr013-use-fetch.md rename to docs/architecture-decisions/adr013-use-node-fetch.md index 43f66fca0f..e65cf0a97b 100644 --- a/docs/architecture-decisions/adr013-use-fetch.md +++ b/docs/architecture-decisions/adr013-use-node-fetch.md @@ -1,10 +1,16 @@ --- id: adrs-adr013 -title: 'ADR013: Proper use of HTTP fetching libraries' +title: 'ADR013: [superseded] Proper use of HTTP fetching libraries' # prettier-ignore -description: Architecture Decision Record (ADR) for the proper use of fetchApiRef, native fetch, and cross-fetch for data fetching. +description: Architecture Decision Record (ADR) for the proper use of fetchApiRef, node-fetch, and cross-fetch for data fetching. --- +:::note Superseded + +This ADR has been superseded by [ADR014](./adr014-use-fetch.md) and no longer applies. + +::: + ## Context Using multiple HTTP packages for data fetching increases the complexity and the @@ -12,13 +18,13 @@ support burden of keeping said package up to date. ## Decision -All code that is executed in Node.js (including backend and CLIs) should use the -native `fetch` for HTTP data fetching. Example: +Backend (node) packages should use the `node-fetch` package for HTTP data +fetching. Example: ```ts +import fetch from 'node-fetch'; import { ResponseError } from '@backstage/errors'; -// this is implicitly global.fetch const response = await fetch('https://example.com/api/v1/users.json'); if (!response.ok) { throw await ResponseError.fromResponse(response); @@ -28,11 +34,12 @@ const users = await response.json(); Frontend plugins and packages should prefer to use the [`fetchApiRef`](https://backstage.io/docs/reference/core-plugin-api.fetchapiref). +It uses `cross-fetch` internally. Example: ```ts import { useApi } from '@backstage/core-plugin-api'; -// Inside some React component... +// Inside some functional React component... const { fetch } = useApi(fetchApiRef); const response = await fetch('https://example.com/api/v1/users.json'); @@ -67,5 +74,6 @@ export class MyClient { ## Consequences -We will gradually transition away from third party `fetch` replacement packages -such as `node-fetch` and others on the Node.js platform. +We will gradually transition away from third party packages such as `axios`, +`got` and others. Once we have transitioned to `node-fetch` we will add lint +rules to enforce this decision. diff --git a/docs/architecture-decisions/adr014-use-fetch.md b/docs/architecture-decisions/adr014-use-fetch.md new file mode 100644 index 0000000000..9fdcb1967d --- /dev/null +++ b/docs/architecture-decisions/adr014-use-fetch.md @@ -0,0 +1,78 @@ +--- +id: adrs-adr014 +title: 'ADR014: Proper use of HTTP fetching libraries' +# prettier-ignore +description: Architecture Decision Record (ADR) for the proper use of fetchApiRef, native fetch, and cross-fetch for data fetching. +--- + +## Context + +Until now we have been recommending the use of `node-fetch` in Node.js contexts +through [ADR013](./adr013-use-node-fetch.md). Since then, Backstage has had its +minimum requirements upgraded to Node.js 20 or newer. The Node.js platform has +established a stable, reliable `undici` based native `fetch` in these versions. +Additionally, there are [some issues](https://github.com/backstage/backstage/issues/24590) +with using third party libraries that only appeared in newer versions of +Node.js. + +## Decision + +All code that is executed in Node.js (including backend and CLIs) should use the +native `fetch` for HTTP data fetching, and `typeof fetch` as the TypeScript type +in code where a `fetch` implementation can be injected or is referred to. +Example: + +```ts +import { ResponseError } from '@backstage/errors'; + +// this is implicitly global.fetch +const response = await fetch('https://example.com/api/v1/users.json'); +if (!response.ok) { + throw await ResponseError.fromResponse(response); +} +const users = await response.json(); +``` + +Frontend plugins and packages should prefer to use the +[`fetchApiRef`](https://backstage.io/docs/reference/core-plugin-api.fetchapiref). + +```ts +import { useApi } from '@backstage/core-plugin-api'; + +// Inside some React component... +const { fetch } = useApi(fetchApiRef); + +const response = await fetch('https://example.com/api/v1/users.json'); +if (!response.ok) { + throw await ResponseError.fromResponse(response); +} +const users = await response.json(); +``` + +Isomorphic packages should have a dependency on the `cross-fetch` package for +mocking and type definitions. Preferably, classes and functions in isomorphic +packages should accept an argument of type `typeof fetch` to let callers supply +their preferred implementation of `fetch`. This lets them adorn the calls with +auth or other information, and track metrics etc, in a cross-platform way. +Example: + +```ts +import crossFetch from 'cross-fetch'; + +export class MyClient { + private readonly fetch: typeof crossFetch; + + constructor(options: { fetch?: typeof crossFetch }) { + this.fetch = options.fetch || crossFetch; + } + + async users() { + return await this.fetch('https://example.com/api/v1/users.json'); + } +} +``` + +## Consequences + +We will gradually transition away from third party `fetch` replacement packages +such as `node-fetch` and others on the Node.js platform. diff --git a/mkdocs.yml b/mkdocs.yml index 1c83fe1c38..748cdd8232 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -226,7 +226,8 @@ nav: - ADR010 - Luxon Date Library: 'architecture-decisions/adr010-luxon-date-library.md' - ADR011 - Plugin Package Structure: 'architecture-decisions/adr011-plugin-package-structure.md' - ADR012 - Use Luxon Locale: 'architecture-decisions/adr012-use-luxon-locale-and-date-presets.md' - - ADR013 - Use node-fetch: 'architecture-decisions/adr013-use-fetch.md' + - ADR013 - Use node-fetch: 'architecture-decisions/adr013-use-node-fetch.md' + - ADR014 - Use fetch: 'architecture-decisions/adr014-use-fetch.md' - FAQ: - Overview: 'faq/index.md' - Product FAQ: 'faq/product.md'