Skip to content
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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

vegerot
Copy link

@vegerot vegerot commented Aug 16, 2024

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

@vegerot
Copy link
Author

vegerot commented Aug 16, 2024

The easiest way to build the extension is just to run npm install instead of npm run build

vegerot added a commit to vegerot/web-vitals that referenced this pull request Aug 19, 2024
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.
@tunetheweb
Copy link
Member

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
@vegerot
Copy link
Author

vegerot commented Aug 19, 2024

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 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants