Skip to content

Add an emitter and event store interface in the tracker core to be used both by the browser and node trackers and use fetch API for making requests (close #1076) - #1337

Merged
Matus Tomlein (matus-tomlein) merged 36 commits into
release/4.0.0from
issue/fetch_api
Aug 29, 2024
Merged

Add an emitter and event store interface in the tracker core to be used both by the browser and node trackers and use fetch API for making requests (close #1076)#1337
Matus Tomlein (matus-tomlein) merged 36 commits into
release/4.0.0from
issue/fetch_api

Conversation

@matus-tomlein

Copy link
Copy Markdown
Contributor

This PR replaces the XMLHttpRequest API in the browser tracker and the got library in the Node tracker with the fetch API in order to make requests to the collector using a common code base.

The changes are:

  1. Add an Emitter in the tracker-core for making requests to the collector. This is used both by the Web and Node trackers.
  2. Add an EventStore interface including an in-memory implementation in the tracker-core. On top of this, the Web trackers implement a local storage based event store. The Node tracker uses the tracker-core implementation.

The new Emitter and EventStore interfaces are Promise-based so can be used to await when the events are stored or sent to the collector. I didn't change any outward facing Web tracker APIs to expose the promises, we can consider that though.

There are various breaking changes, I'll need to make a full list and publish in docs.

As part of the work, I also had to update the Node.JS version used for the build – I set the supported versions to >=18.0.0 <25.0.0.

@bundlemon

bundlemon Bot commented Aug 20, 2024

Copy link
Copy Markdown

BundleMon

Files added (6)
Status Path Size Limits
trackers/javascript-tracker/dist/sp.js
+27.69KB 30KB / +10%
libraries/browser-tracker-core/dist/index.mod
ule.js
+22.25KB 25KB / +10%
libraries/tracker-core/dist/index.module.js
+19.12KB 20KB / +10%
trackers/browser-tracker/dist/index.umd.min.j
s
+16.83KB 20KB / +10%
trackers/javascript-tracker/dist/sp.lite.js
+16.81KB 20KB / +10%
trackers/browser-tracker/dist/index.module.js
+3.49KB 5KB / +10%

Total files change +106.18KB 0%

Final result: ✅

View report in BundleMon website ➡️


Current branch size history

@jethron Jethro Nederhof (jethron) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome changes! Mostly LGTM, just a few things.

Comment thread trackers/javascript-tracker/test/functional/activityCallback.test.ts Outdated
Comment thread trackers/node-tracker/src/index.ts Outdated
Comment thread trackers/node-tracker/src/tracker.ts Outdated
Comment thread rush.json
Comment thread trackers/node-tracker/src/tracker.ts Outdated
Comment thread trackers/browser-tracker/test/tracker.test.ts Outdated
Comment thread libraries/tracker-core/test/emitter/emitter_event.test.ts Outdated
Comment thread libraries/tracker-core/test/emitter/emitter_event.test.ts
Comment thread libraries/browser-tracker-core/src/tracker/out_queue.ts Outdated
Comment thread libraries/browser-tracker-core/src/tracker/out_queue.ts

@jethron Jethro Nederhof (jethron) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Appropriately epic PR for 1337, great work!

(Pretty sure tests just need to be more async friendly because it's a whole new task rather than microtask:)

       it('should fire on a successful request', async () => {
         const callbacks: EventBatch[] = [];
-        const onSuccess = (e: EventBatch) => {
-          callbacks.push(e);
-        };
 
-        const postQueue = createQueue({ method, onSuccess });
-        await postQueue.enqueueRequest(request);
+        await new Promise<void>(async (resolve) => {
+          const onSuccess = (e: EventBatch) => {
+            callbacks.push(e);
+            resolve();
+          };
+
+          const postQueue = createQueue({ method, onSuccess });
+          await postQueue.enqueueRequest(request);
+        });
 
         expect(requests).toHaveLength(1);
         expect(eventStore.addCount()).toEqual(1);

@matus-tomlein

Copy link
Copy Markdown
Contributor Author

Thanks a lot for the in depth review Jethro Nederhof (@jethron), that was super helpful!

@matus-tomlein
Matus Tomlein (matus-tomlein) merged commit 72731a4 into release/4.0.0 Aug 29, 2024
@matus-tomlein
Matus Tomlein (matus-tomlein) deleted the issue/fetch_api branch August 29, 2024 06:57
Matus Tomlein (matus-tomlein) added a commit that referenced this pull request Oct 25, 2024
…kers and refactor emitter and event store interfaces (close #1076)

PR #1337
Matus Tomlein (matus-tomlein) added a commit that referenced this pull request Oct 28, 2024
…kers and refactor emitter and event store interfaces (close #1076)

PR #1337
Matus Tomlein (matus-tomlein) added a commit that referenced this pull request Oct 28, 2024
…kers and refactor emitter and event store interfaces (close #1076)

PR #1337
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants