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

Add frontend hook #187

Merged
merged 9 commits into from
Nov 2, 2020
Merged

Add frontend hook #187

merged 9 commits into from
Nov 2, 2020

Conversation

Widcket
Copy link
Contributor

@Widcket Widcket commented Nov 2, 2020

Description

This PR adds a frontend hook to fetch the User Profile from the React side of the app. The sample apps have been updated to use the provided hook.

Testing

  • This change adds test coverage for new/changed/fixed functionality

Checklist

  • I have added documentation for new/changed functionality in this PR or in auth0.com/docs
  • All active GitHub checks for tests, formatting, and security are passing
  • The correct base branch is being used, if not master

@Widcket Widcket requested a review from adamjmcgrath November 2, 2020 07:50
@Widcket Widcket added review:medium Medium review CH: Added PR is adding feature or functionality labels Nov 2, 2020
@adamjmcgrath adamjmcgrath self-assigned this Nov 2, 2020
examples/typescript-example/components/with-auth.tsx Outdated Show resolved Hide resolved
// eslint-disable-next-line import/no-extraneous-dependencies
import React, { ReactElement, useState, useEffect, useContext, createContext } from 'react';

declare const fetch: any;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add lib: ['dom'] to the tsconfig, then you wont need to declare fetch https://github.com/auth0/auth0-react/blob/master/tsconfig.json#L4

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added "browser": true to eslint config. BTW, the fetch polyfill comes bundled with Next.js 9.4.0+. Currently the SDK requires 9.3.6. If we're going to keep that minimum version, we need to use node-fetch or similar here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's go with 9.4 for min version

@adamjmcgrath
Copy link
Contributor

lgtm 👍 just a couple of TS things

We can look at the failing tests later

@Widcket Widcket marked this pull request as ready for review November 2, 2020 15:00
@Widcket Widcket requested a review from a team as a code owner November 2, 2020 15:00
@Widcket Widcket requested a review from adamjmcgrath November 2, 2020 15:00
@adamjmcgrath adamjmcgrath merged commit 50d7a1b into beta Nov 2, 2020
@adamjmcgrath adamjmcgrath deleted the feature/frontend-hook branch November 2, 2020 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CH: Added PR is adding feature or functionality review:medium Medium review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants