GoogleChrome / web-vitals

Essential metrics for a healthy site.
https://web.dev/vitals
Apache License 2.0
7.63k stars 418 forks source link

fix: distribute tsconfig and rollup config files #522

Closed vegerot closed 3 months ago

vegerot commented 3 months ago

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:

Motivation:

I am currently working on a patch 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 --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 commented 3 months ago

I would have thought if you only wanted to build this library, then you'd download the source (i.e. git clone the repo) and then build. So you'd have the tsconfig.json and rollup.config.js files.

Alternatively, if you wanted to to build this as part of a larger app, you'd npm install is as a dependency and use that larger project's typescript and build options.

Is it usually expected to build just the package from npm install? Not sure I've come across that before for a JS library...

vegerot commented 3 months ago

Thanks for the quick reply!

Is it usually expected to build just the package from npm install? Not sure I've come across that before for a JS library...

I don't know. My feeling was that if we distribute the src/, the expectation is that users should be able to build it. I understand your feeling that users should just write their own build configuration.

What if the web-vitals package's build steps were more complicated and we still distributed src/? Would it be expected that users reimplement the whole build configuration in order to use the src/ files we already distribute? I get that in this case the build steps are simple so writing a script in the Web Vitals Chrome extension wouldn't be hard.

vegerot commented 3 months ago

As a follow-up, what would you suggest I do for https://github.com/GoogleChrome/web-vitals-extension/pull/184 ? Do you think I should git clone this package instead of npm installing it?

vegerot commented 3 months ago

idea: can we publish a web-vitals@soft-nav version of this package on npm? That would make the Extension patch the simplest

tunetheweb commented 3 months ago

It already is published there! And I try to keep it up to date as we make changes to the main branch.

vegerot commented 3 months ago

It already is published there! And I try to keep it up to date as we make changes to the main branch.

🤦🏾‍♀️ I swear this was the first place I looked, and after not finding it, I decided to do this more complicated solution. Sorry I can't read!