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

fix(usage): add chrome namespace to Window and add usage directions, fixes #17 #34

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

Conversation

MichaelSolati
Copy link
Contributor

  1. Add dist/global.d.ts which takes the build output and adds it to the Window interface.
  2. Update dist/README.md to document how to include this lib into a projects global typings.
  3. Tell .gitignore to include dist/global.d.ts.
  4. Update dist/package.template.json with some keywords for search on npm.

See this video for a little better explenation. I also didn't want to modify any of the existing files so that we don't break the chrome types being used for dcc.

@codecov
Copy link

codecov bot commented Mar 2, 2022

Codecov Report

Merging #34 (b93130d) into main (dfe80f3) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main      #34   +/-   ##
=======================================
  Coverage   35.47%   35.47%           
=======================================
  Files          21       21           
  Lines        3986     3986           
  Branches      183      183           
=======================================
  Hits         1414     1414           
  Misses       2572     2572           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dfe80f3...b93130d. Read the comment docs.

@MichaelSolati
Copy link
Contributor Author

@devnook ptal

@olso-nordsec
Copy link

this will not force it to be a primary resolution in case some third party package installed @types/chrome

@MichaelSolati
Copy link
Contributor Author

@olso-nordsec without being in the @types namespace this isn't an easy (possible?) thing to achieve. Additionally, merging with @DefinitelyTyped is something that makes sense to do. The way the typed libraries work with DefinitelyTyped and this lib work are very different. It would be interesting if there was a @types/* lib that digests what's generated and exports it would be interesting, but that depends on the bandwidth of the Chrome team.

@olso-nordsec
Copy link

For now, I'm using patch-package

diff --git a/node_modules/chrome-types/index.d.ts b/node_modules/chrome-types/index.d.ts
index 4438d02..41e7899 100644
--- a/node_modules/chrome-types/index.d.ts
+++ b/node_modules/chrome-types/index.d.ts
@@ -1,3 +1,4 @@
+declare module 'chrome-types' {
 /**
  * Copyright 2022 Google LLC
  *
@@ -28738,3 +28739,4 @@ declare namespace chrome {
     ): void;
   }
 }
+}

and then using it as

import type { chrome } from 'chrome-types';

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.

3 participants