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

Issue setting up Strategy [in TypeScript] #25

Closed
optojack opened this issue Dec 16, 2022 · 8 comments · Fixed by #26 or #32
Closed

Issue setting up Strategy [in TypeScript] #25

optojack opened this issue Dec 16, 2022 · 8 comments · Fixed by #26 or #32
Labels
bug Something isn't working

Comments

@optojack
Copy link

optojack commented Dec 16, 2022

Hello, I am trying to setup an app to use this package but I am having issues setting the Strategy; specifically the following:

passport.use(
  new Strategy(
    {
      clientID: env.TWITTER_API_CLIENT_ID,
      clientSecret: env.TWITTER_API_CLIENT_SECRET,
      clientType: 'confidential',
      callbackURL: 'http://localhost:3000/auth/twitter/callback',
    },
    function (accessToken, refreshToken, profile, done) {
      return done(null, profile);
    }
  )
);

I have most commonly seen 2 errors between trying different things to resolve the issues:
1.

Property 'authenticate' is missing in type 'import(".../node_modules/@superfaceai/passport-twitter-oauth2/dist/strategy").Strategy' but required in type 'import(".../node_modules/@types/passport/index").Strategy'.
'Strategy' cannot be used as a value because it was exported using 'export type'.

I will add that I am using typescript (not sure if that should be causing these issues though) and I am working off the example code here

Thanks in advance for any help!!

@jnv
Copy link
Collaborator

jnv commented Dec 16, 2022

Thanks for the report. It's possible the current CommonJS exports have some troubles with TypeScript since I focused on keeping the backwards compatibility of default + named export.

Could you please add a rest of your example? Notably, are you using require or import to import the strategy? And in the former case, did you try using import?

@Push-Stack
Copy link

Push-Stack commented Dec 20, 2022

I am working on a Nest JS project.
I am getting the same error Strategy' cannot be used as a value because it was exported using 'export type.

import { Strategy, Profile } from '@superfaceai/passport-twitter-oauth2';

@Injectable()
export class TwitterStrategy extends PassportStrategy(Strategy, 'twitter') {
  constructor(
    @Inject(TwitterConfig.KEY) twitter: ConfigType<typeof TwitterConfig>,
  ) {
    super({
      clientID: twitter.clientId,
      clientSecret: twitter.clientSecret,
      callbackURL: twitter.callbackUrl,
      scope: ['tweet.read', 'users.read', 'offline.access'],
    });
  }

  async verify(accessToken: string, refreshToken: string, profile: Profile) {
    console.log(accessToken, refreshToken, profile);

    return { accessToken, refreshToken, profile };
  }
}

@janhalama
Copy link
Collaborator

I can confirm that there is an issue in TypeScript support. I have opened PR with fixes.

@jnv jnv linked a pull request Dec 21, 2022 that will close this issue
@jnv jnv changed the title Issue setting up Strategy Issue setting up Strategy [in TypeScript] Dec 21, 2022
@jnv jnv added the bug Something isn't working label Dec 21, 2022
@janhalama
Copy link
Collaborator

@JackLit, @Push-Stack We've just released @superfaceai/passport-twitter-oauth2 v1.2.1 with TypeScript fixes.

@kazuemon
Copy link

I am using @superfaceai/passport-twitter-oauth2@1.2.2 and continue to have this problem.
Installing @types/passport-oauth2 at the same time solved the problem.

@jnv
Copy link
Collaborator

jnv commented Dec 24, 2022

@kazuemon Thanks for the report. I will reopen this issue.

I see three options how to address it:

  1. Add missing dependency on @types/passport-oauth2; easiest, but it has multiple transitive dependencies and it would add extra weight for non-TS users.
  2. Make @types/passport-oauth2 an optional dependency and document it needs to be installed.
  3. Pull in just the types we need from @types/passport-oauth2 and its transitive dependencies.

Just that types package adds 3,5 MB install size which is why I'm not quite happy with slapping it on as an extra dependency.

@jnv
Copy link
Collaborator

jnv commented Jan 2, 2023

I decided to go with optionalDependencies in #32. While it means the types get installed for non-TS users by default, it should be possible to omit them.

@jnv jnv closed this as completed in #32 Jan 5, 2023
@jnv
Copy link
Collaborator

jnv commented Jan 13, 2023

Hey folks, I've released v1.2.3 with optional @types dependencies today, so hopefully it should fix this issue for good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment