-
Notifications
You must be signed in to change notification settings - Fork 106
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
feat: add experimental support for soft navigations #184
base: main
Are you sure you want to change the base?
Conversation
The easiest way to build the extension is just to run |
Summary: We currently distribute the source code for the project, but not the configuration files, meaning that users cannot build the project. This commit adds the tsconfig.json and rollup.config.js files to the distributed package. Test Plan: - Run `npm run build` and ensure that the project builds successfully - In another project add this package as a dependency and run `npm install && cd node_modules/web-vitals && npm install --ignore-scripts && npm run build` and ensure that the project builds successfully Motivation: I am currently working on a [patch](GoogleChrome/web-vitals-extension#184) for the web vitals Chrome extension and need to use a development build of this web-vitals package. To do that, I want to make a patch like ```diff diff --git a/package.json b/package.json --- a/package.json +++ b/package.json @@ -13,7 +13,7 @@ "private": true, "scripts": { "lint": "npx eslint src --fix", - "build": "npm install; cp node_modules/web-vitals/dist/web-vitals.attribution.js src/browser_action/web-vitals.js" + "build": "npm install && (cd node_modules/web-vitals/ && npm install --ignore-scripts && npm run build) && cp node_modules/web-vitals/dist/web-vitals.attribution.js src/browser_action/web-vitals.js" }, "devDependencies": { "babel-eslint": "^10.1.0", @@ -21,6 +21,10 @@ "eslint-config-google": "^0.14.0" }, "dependencies": { - "web-vitals": "^4.0.0" + "web-vitals": "git://github.com/GoogleChrome/web-vitals.git#soft-navs" + }, ``` however, the `npm run build` part fails because we don't distribute `tsconfig.json` and `rollup.config.js`. This commit fixes that.
Thank you for the work on this. And I understand why this would be useful, but while soft-nav support is experimental (and particularly at the moment when it's not being run in the origin trial while we're making changes to the implementation), I'd prefer not to merge this change and include soft nav support in the official extension (even if it's marked as experimental and disabled in the cases you describe above). We can reevaluate this when a new origin trial is launched and this becomes more generally available. Happy to keep this branch open in the meantime so people can install this build if they wish, but just wanted you to make you aware why we're unlikely to accept this PR for now. |
Summary: Recent versions of Chromium have added support for tracking web vitals metrics across soft navigations. This change adds support for tracking these metrics in the extension. TODO: - [ ] document the new `reportSoftNavs` option in the README `web-vitals` package - [ ] Hide the option in the options page if one of the following is false: 1. The browser supports the `soft-navigation` entry type 2. The `web-vitals` package has been built with the `soft-navs` branch
Thanks for taking a look. That makes sense. I'm working on this so my team can use it. They're already using this PR to grab the code from, so this works for them too 🙂 |
Summary:
Recent versions of Chromium have added support for tracking web vitals metrics
across soft navigations. This change adds support for tracking these metrics in
the extension.
TODO:
reportSoftNavs
option in the READMEweb-vitals
packagesoft-navigation
entry typeweb-vitals
package has been built with thesoft-navs
branch