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: make it webpackable #371

Merged
merged 87 commits into from
Jan 8, 2019
Merged

feat: make it webpackable #371

merged 87 commits into from
Jan 8, 2019

Conversation

alexander-fenster
Copy link
Contributor

@alexander-fenster alexander-fenster commented Jun 1, 2018

BREAKING CHANGE: This change makes google-auth-library OAuth2 functionality available for using in browsers (using webpack).

To reduce the bundle size, we use SubtleCrypto everywhere where Node.js crypto module is used. Since all SubtleCrypto methods return promises, we made two breaking changes to OAuth2Client class:

  • The OAuth2Client.generateCodeVerifier method has been replaced by the asynchronous async generateCodeVerifierAsync method
  • The OAuth2Client.verifySignedJwtWithCerts method has been replaced by the async verifySignedJwtWithCertsAsync method

@alexander-fenster alexander-fenster requested review from JustinBeckwith and a team June 1, 2018 22:40
@alexander-fenster alexander-fenster added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Jun 1, 2018
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jun 1, 2018
@codecov-io
Copy link

codecov-io commented Jun 1, 2018

Codecov Report

❗ No coverage uploaded for pull request base (master@48db9eb). Click here to learn what that means.
The diff coverage is 79.82%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #371   +/-   ##
=========================================
  Coverage          ?   93.03%           
=========================================
  Files             ?       17           
  Lines             ?     1033           
  Branches          ?      213           
=========================================
  Hits              ?      961           
  Misses            ?       72           
  Partials          ?        0
Impacted Files Coverage Δ
src/crypto/node/crypto.ts 100% <100%> (ø)
src/webpack.ts 100% <100%> (ø)
src/transporters.ts 96.15% <100%> (ø)
src/crypto/browser/crypto.ts 29.16% <29.16%> (ø)
src/crypto/crypto.ts 85.71% <85.71%> (ø)
src/auth/oauth2client.ts 93.89% <92.06%> (ø)

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 48db9eb...d2da190. Read the comment docs.

@alexander-fenster alexander-fenster changed the title DO NOT MERGE: make it webpackable DO NOT MERGE: feat: make it webpackable Jun 2, 2018
src/auth/oauth2client.ts Outdated Show resolved Hide resolved
src/auth/oauth2client.ts Outdated Show resolved Hide resolved
src/auth/oauth2client.ts Show resolved Hide resolved
src/auth/oauth2client.ts Outdated Show resolved Hide resolved
src/auth/oauth2client.ts Outdated Show resolved Hide resolved
src/crypto/browser/crypto.ts Outdated Show resolved Hide resolved
src/crypto/browser/jwkverifier.ts Outdated Show resolved Hide resolved
webpack.config.js Outdated Show resolved Hide resolved
tsconfig.json Show resolved Hide resolved
webpack.config.js Outdated Show resolved Hide resolved
@JustinBeckwith
Copy link
Contributor

I know you're on it, but I think we should make sure to include browser tests :)

@alexander-fenster
Copy link
Contributor Author

@JustinBeckwith One more look? :)

@alexander-fenster
Copy link
Contributor Author

Re: changelog

From what I understand, for the basic usage of auth library, there should be no changes. Two functions were removed from OAuth2Client: generateCodeVerifier and verifySignedJwtWithCerts (replaced with generateCodeVerifierAsync and verifySignedJwtWithCertsAsync).

I'm trying to prepare a webpacked version of google-api-nodejs-client with this latest state of auth library, I'll link it to this PR when it works.

browser-test/test.oauth2.ts Outdated Show resolved Hide resolved
browser-test/test.oauth2.ts Outdated Show resolved Hide resolved
browser-test/test.oauth2.ts Outdated Show resolved Hide resolved
browser-test/test.oauth2.ts Outdated Show resolved Hide resolved
browser-test/test.oauth2.ts Outdated Show resolved Hide resolved
browser-test/test.oauth2.ts Outdated Show resolved Hide resolved
browser-test/test.oauth2.ts Outdated Show resolved Hide resolved
src/auth/oauth2client.ts Outdated Show resolved Hide resolved
src/crypto/browser/crypto.ts Outdated Show resolved Hide resolved
test/test.oauth2.ts Show resolved Hide resolved
@JustinBeckwith
Copy link
Contributor

@alexander-fenster this is looking awesome. After you add details on the breaking changes to the description, I can approve this and we can start cranking on v3 💃

@alexander-fenster
Copy link
Contributor Author

@JustinBeckwith I updated the PR description above.

@JustinBeckwith
Copy link
Contributor

@alexander-fenster I'll let you do the honors of merging :)

@alexander-fenster alexander-fenster merged commit cf3aedc into master Jan 8, 2019
@alexander-fenster
Copy link
Contributor Author

I. Cannot. Believe. It. Is. Merged.

Thanks @JustinBeckwith :)

@dmytrobanasko
Copy link

Facing an issue after switching from webbbpackkk branch to latest release (actually, after axios -> gaxios)
@alexander-fenster, did you try it with new client ?

@alexander-fenster
Copy link
Contributor Author

Let's discuss it there to keep the conversation in one place :) Thanks for your report!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.