fix: move from event-source-polyfill to fetch-event-source #24861

alexef · 2024-05-22T07:31:56Z

Hey, I just made a Pull Request!

Replace event-source-polyfill with @microsoft/fetch-event-source so we can pass our fetchApi implementation.

✔️ Checklist

A changeset describing the change and affected packages. (more info)
Added or updated documentation
Tests for new functionality and regression tests for bug fixes
Screenshots attached (for UI changes)
All your commits have a Signed-off-by line in the message. (more info)

backstage-goalie · 2024-05-22T07:32:26Z

Changed Packages

Package Name	Package Path	Changeset Bump	Current Version
@backstage/plugin-scaffolder	plugins/scaffolder	patch	`v1.20.2-next.1`
@backstage/plugin-techdocs	plugins/techdocs	patch	`v1.10.6-next.0`

alexef · 2024-05-22T09:52:39Z

Actually it seems to work fine (using yarn dev in the root, I can see logs fetched via event source), and I can also confirm that fetchApi is being used.

alexef · 2024-05-22T12:58:07Z

Also tested the scaffolder, and events stream just like before:

Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com>

aramissennyeydd

Nice!

.changeset/empty-avocados-tease.md

plugins/techdocs/src/client.ts

Co-authored-by: Aramis Sennyey <159921952+aramissennyeydd@users.noreply.github.com> Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com>

benjdlambert · 2024-05-27T12:38:21Z

plugins/scaffolder/src/api.ts

-          });
-          eventSource.addEventListener('error', event => {
-            subscriber.error(event);
+          fetchEventSource(url, {


Does this automatically cleanup? We seem to missing the eventSource.close from before.

I assume it does. It has an onclose hook, but no explicit call to close in the documentation: https://github.com/Azure/fetch-event-source?tab=readme-ov-file#usage

It uses a signal instead, which is the more modern API. We should implement it using that instead.

I wonder if we should have the option of passing in an AbortController, and default to creating one if one is not provided? Maybe can save that for a followup though..

plugins/scaffolder/src/api.ts

benjdlambert

Hey 👋 thanks for this, just a few small things! 🙏

benjdlambert · 2024-05-27T12:39:36Z

plugins/techdocs/api-report.md

@@ -465,7 +464,6 @@ export class TechDocsStorageClient implements TechDocsStorageApi_2 {
  constructor(options: {
    configApi: Config;
    discoveryApi: DiscoveryApi;
-    identityApi: IdentityApi;


Thinking that we should mark this as @deprecated and make it optional and just skip using it rather than breaking change. It would only affect people who provide a custom factory to the app for creating a techdocsApiRef but it's still technically breaking.

I'm not 100% sure the way I marked it as deprecated is fine (couldn't find a similar example). Can you check?

It needs to use a doc comment, i.e. /** @deprecated ... */. Let's keep the IdentityApi type as well, instead of any

I did the comment, but somehow I can't make it show up in the generated api-report.md.

any hints are welcome

plugins/techdocs/src/client.ts

Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com>

Rugvip

Sweet, thank you! 👍

Just a couple of small fixes

Rugvip · 2024-05-28T13:04:36Z

plugins/scaffolder/src/api.ts

-          });
-          eventSource.addEventListener('error', event => {
-            subscriber.error(event);
+          fetchEventSource(url, {


It uses a signal instead, which is the more modern API. We should implement it using that instead.

Rugvip · 2024-05-28T13:06:28Z

plugins/techdocs/api-report.md

@@ -465,7 +464,6 @@ export class TechDocsStorageClient implements TechDocsStorageApi_2 {
  constructor(options: {
    configApi: Config;
    discoveryApi: DiscoveryApi;
-    identityApi: IdentityApi;


It needs to use a doc comment, i.e. /** @deprecated ... */. Let's keep the IdentityApi type as well, instead of any

plugins/techdocs/src/plugin.ts

Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com>

github-actions · 2024-05-28T16:58:09Z

Uffizzi Ephemeral Environment - Virtual Cluster

Your cluster pr-24861 was successfully created. Learn more about Uffizzi virtual clusters
To connect to this cluster, follow these steps:

Download and install the Uffizzi CLI from https://docs.uffizzi.com/install
Login to Uffizzi, then select the backstage account and project:

uffizzi login

Select an account: 
  ‣ backstage
    jdoe

Select a project or create a new project: 
  ‣ backstage-6783521

Update your kubeconfig: uffizzi cluster update-kubeconfig pr-24861 --kubeconfig=[PATH_TO_KUBECONFIG]
After updating your kubeconfig, you can manage your cluster with kubectl, kustomize, helm, and other tools that use kubeconfig files: kubectl get namespace --kubeconfig [PATH_TO_KUBECONFIG]

Access the backstage endpoint at https://backstage-default-pr-24861-c5600.uclusters.app.uffizzi.com

Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com>

alexef · 2024-05-28T19:02:52Z

seems like e2e is broken now. I tried merging master onto my branch, no success to fix them. I doubt the failure relates to the code, will try to reproduce it locally

vinzscam

this is amazing, thanks for cleaning it up 🙏

.changeset/empty-avocados-tease.md

Co-authored-by: Vincenzo Scamporlino <vincenzos@spotify.com> Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com>

Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com>

alexef · 2024-06-03T07:29:01Z

@Rugvip / @benjdlambert can you have another look?

Rugvip

Nice, thank you! 👍 🎉

Resolved

github-actions · 2024-06-03T13:01:01Z

Thank you for contributing to Backstage! The changes in this pull request will be part of the 1.28.0 release, scheduled for Tue, 18 Jun 2024.

github-actions bot added the area:techdocs Related to the TechDocs Project Area label May 22, 2024

alexef mentioned this pull request May 22, 2024

Use fetch middlewares in event-source-polifyll (re: #23015) #24820

Closed

5 tasks

github-actions bot added the area:scaffolder Everything and all things related to the scaffolder project area label May 22, 2024

alexef marked this pull request as ready for review May 22, 2024 12:50

alexef requested review from a team and backstage-service as code owners May 22, 2024 12:50

alexef requested review from Rugvip, vinzscam and benjdlambert May 22, 2024 12:50

alexef changed the title ~~Try to switch away from polyfill~~ Move from event-source-polyfill to fetch-event-source May 22, 2024

alexef added 5 commits May 22, 2024 15:01

Try to switch away from polyfill

1593991

Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com>

Update unit tests

f1c280b

Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com>

Cleanup and fix build

db76476

Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com>

Fix build. Move scaffolder away from polyfill

147f11f

Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com>

Add changeset

cbebad1

Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com>

alexef force-pushed the new-events-implementation branch from 2799245 to cbebad1 Compare May 22, 2024 13:01

alexef changed the title ~~Move from event-source-polyfill to fetch-event-source~~ fix: move from event-source-polyfill to fetch-event-source May 22, 2024

aramissennyeydd approved these changes May 22, 2024

View reviewed changes

.changeset/empty-avocados-tease.md Outdated Show resolved Hide resolved

plugins/techdocs/src/client.ts Show resolved Hide resolved

Update .changeset/empty-avocados-tease.md

79605f8

Co-authored-by: Aramis Sennyey <159921952+aramissennyeydd@users.noreply.github.com> Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com>

alexef mentioned this pull request May 27, 2024

🐛 Bug Report: Unable to use a different name for the Authorization token #23015

Closed

2 tasks

benjdlambert reviewed May 27, 2024

View reviewed changes

benjdlambert previously requested changes May 27, 2024

View reviewed changes

address PR comments

a159480

Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com>

alexef requested a review from benjdlambert May 27, 2024 14:39

Rugvip requested changes May 28, 2024

View reviewed changes

update pr comments

b7db436

Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com>

alexef requested a review from Rugvip May 28, 2024 16:50

Merge branch 'master' into new-events-implementation

7cc45e0

Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com>

vinzscam requested changes May 29, 2024

View reviewed changes

.changeset/empty-avocados-tease.md Outdated Show resolved Hide resolved

Update .changeset/empty-avocados-tease.md

edc1801

Co-authored-by: Vincenzo Scamporlino <vincenzos@spotify.com> Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com>

alexef requested a review from vinzscam May 30, 2024 00:25

Fix unittests

9d54ebf

Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com>

vinzscam approved these changes May 30, 2024

View reviewed changes

Rugvip approved these changes Jun 3, 2024

View reviewed changes

Rugvip merged commit 42f1865 into backstage:master Jun 3, 2024
38 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: move from event-source-polyfill to fetch-event-source #24861

fix: move from event-source-polyfill to fetch-event-source #24861

alexef commented May 22, 2024 •

edited

backstage-goalie bot commented May 22, 2024 •

edited

alexef commented May 22, 2024 •

edited

alexef commented May 22, 2024

aramissennyeydd left a comment

benjdlambert May 27, 2024

alexef May 27, 2024

Rugvip May 28, 2024

alexef May 28, 2024

benjdlambert May 30, 2024

benjdlambert left a comment

benjdlambert May 27, 2024

alexef May 27, 2024

Rugvip May 28, 2024

alexef May 29, 2024

Rugvip left a comment

Rugvip May 28, 2024

Rugvip May 28, 2024

github-actions bot commented May 28, 2024 •

edited

alexef commented May 28, 2024

vinzscam left a comment

alexef commented Jun 3, 2024

Rugvip left a comment

github-actions bot commented Jun 3, 2024

fix: move from event-source-polyfill to fetch-event-source #24861

fix: move from event-source-polyfill to fetch-event-source #24861

Conversation

alexef commented May 22, 2024 • edited

Hey, I just made a Pull Request!

✔️ Checklist

backstage-goalie bot commented May 22, 2024 • edited

Changed Packages

alexef commented May 22, 2024 • edited

alexef commented May 22, 2024

aramissennyeydd left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benjdlambert left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Rugvip left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented May 28, 2024 • edited

Uffizzi Ephemeral Environment - Virtual Cluster

alexef commented May 28, 2024

vinzscam left a comment

Choose a reason for hiding this comment

alexef commented Jun 3, 2024

Rugvip left a comment

Choose a reason for hiding this comment

github-actions bot commented Jun 3, 2024

alexef commented May 22, 2024 •

edited

backstage-goalie bot commented May 22, 2024 •

edited

alexef commented May 22, 2024 •

edited

github-actions bot commented May 28, 2024 •

edited