-
Notifications
You must be signed in to change notification settings - Fork 218
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Create performance navigation timing plugin (close #1171) #1172
Create performance navigation timing plugin (close #1171) #1172
Conversation
BundleMonFiles added (6)
Total files change +94.09KB 0% Final result: ✅ View report in BundleMon website ➡️ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! Just thinking if we can reduce the size of the entity by removing 0 properties.
plugins/browser-plugin-performance-navigation-timing/src/contexts.ts
Outdated
Show resolved
Hide resolved
plugins/browser-plugin-performance-navigation-timing/src/index.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM pending schema finalisation.
plugins/browser-plugin-performance-navigation-timing/src/contexts.ts
Outdated
Show resolved
Hide resolved
plugins/browser-plugin-performance-navigation-timing/src/contexts.ts
Outdated
Show resolved
Hide resolved
6214b35
to
7d6a8f8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
## Maintainer quick start | ||
|
||
Part of the Snowplow JavaScript Tracker monorepo. | ||
Build with [Node.js](https://nodejs.org/en/) (14 or 16) and [Rush](https://rushjs.io/). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sidenote: Node 14 hits EOL in like 10 days, do we have plans to support current LTS (18) for all our stuff in the near future?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For our build systems, we should make sure Rush supports Node.js 18 properly, but from my experience we can just test it out and change our docs as required.
For the targets, in v4 we ditched Node.js 12 but we will be keeping Node.js 14 as the minimum still. On that, we need to make sure we also support Node.js 18 and Node.js 20 on our Node.js tracker.
2b2187c
to
c9860b8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! 👍
c9860b8
to
98f0eb2
Compare
Add a new
@snowplow/browser-plugin-performance-navigation-timing
plugin which will pull information from PerformanceNavigationTiming API.Currently this works like the
performance-timing
plugin, attaching the context in each event after activation.