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

fastifyPassport.initialize is not a function #36

Closed
matt212 opened this issue Dec 7, 2020 · 17 comments
Closed

fastifyPassport.initialize is not a function #36

matt212 opened this issue Dec 7, 2020 · 17 comments

Comments

@matt212
Copy link

matt212 commented Dec 7, 2020

🐛 Bug Report

A clear and concise description of what the bug is.

To Reproduce

Steps to reproduce the behavior:

// Paste your code here
const fastify = require('fastify')();
const fastifyPassport= require('fastify-passport')
const fastifySecureSession= require('fastify-secure-session')
// set up secure sessions for fastify-passport to store data in
fastify.register(fastifySecureSession, { key: fs.readFileSync(path.join(__dirname, "secret-key")) });
// initialize fastify-passport and connect it to the secure-session storage. Note: both of these plugins are mandatory.
fastify.register(fastifyPassport.initialize());
fastify.register(fastifyPassport.secureSession());
console.log(fastifyPassport);

Error FastifyPassport.initialize is not a function

image

Expected behavior

A clear and concise description of what you expected to happen.

fastifyPassport.initialize should initialize passport
also on same note when someone console `fastifyPassport`
below appears
{
  Strategy: [Getter],
  default: Authenticator {
    key: 'passport',
    userProperty: 'user',
    strategies: { session: [SessionStrategy] },
    serializers: [],
    deserializers: [],
    infoTransformers: [],
    sessionManager: SecureSessionManager {
      key: 'passport',
      serializeUser: [Function: bound serializeUser] AsyncFunction
    }
  }
}

Your Environment

  • node version: v14.15.1
  • fastify version: >=^3.9.1
  • os: windows-wsl-ubuntu
@airhorns
Copy link
Member

airhorns commented Dec 10, 2020

Thanks for the bug report. Super weird. Which version of fastify-passport are you experiencing this on?

@matt212
Copy link
Author

matt212 commented Dec 10, 2020

Hi @airhorns
So the version as per my install package.json is ^0.1.0
Also on another note Since I was pressed for time I opted by writing up a custom login strategies for my requirement using Fastify decorators and JWT.
Thanks again for Response

@airhorns
Copy link
Member

Ok, thanks, and which typescript version are you running on?

@matt212
Copy link
Author

matt212 commented Dec 10, 2020

Hi @airhorns
No using typescript

@airhorns
Copy link
Member

Oh whoops right, .js files. Are you using a packager like webpack or parcel or something like that? What I'm trying to understand is why the way the code is exported works for some folks and not for you and my suspicion is that something in the middle is breaking the exports.

@airhorns
Copy link
Member

@fox1t I think but am not sure that this is related to #6 where esModuleInterop was disabled. Requiring this package in node returns an object with the default key and the Strategy key, instead of just the value of the default key. I think this is because we're mixing a default export and named exports. Is this something you have experienced in the other packages where you disabled esModuleInterop?

I also don't yet understand why this is working for some people if requiring is broken like this.

@matt212
Copy link
Author

matt212 commented Dec 10, 2020

@airhorns yeah I am not using any packager.
what I can do is paste my
app.txt
app.js and package.json here
package.txt
would that help with your analysis?
Anything else do you require Please let me know
Thanks 👍

@Melodyn
Copy link

Melodyn commented Dec 12, 2020

Hi! My study project has esModuleInterop enabled, but there is the same import problem. I would like you to split imports (import Strategy from 'fastify-passport/Strategy') or make an import of the global object, for example: export default {passport, Strategy}. Airbnb does not recommend mixing imports: https://github.com/benmosher/eslint-plugin-import/blob/master/docs/rules/no-default-export.md

@fox1t
Copy link
Member

fox1t commented Dec 15, 2020

@fox1t I think but am not sure that this is related to #6 where esModuleInterop was disabled. Requiring this package in node returns an object with the default key and the Strategy key, instead of just the value of the default key. I think this is because we're mixing a default export and named exports. Is this something you have experienced in the other packages where you disabled esModuleInterop?

I also don't yet understand why this is working for some people if requiring is broken like this.

AS far as I know, esModuleInterop should not create this problem. However, it is worth spending some time to dig this down.
@matt212 can you provide a repository that has the minimum reproduction of this issue? It would be helpful for us to start from there.

matt212 added a commit to matt212/Nodejs_Postgresql_VanillaJS_Fastify that referenced this issue Dec 15, 2020
@fox1t
Copy link
Member

fox1t commented Dec 15, 2020

Looking better on how we export this https://github.com/fastify/fastify-passport/blob/main/src/index.ts#L5 shows that we are just exporting a default property for CJS contexts. So every user that uses CJS context should import this like const fastifyPassport= require('fastify-passport').default in order to make it work. I suggest using what I call ".js bridge file" that makes TS modules work both in CJS context and in ESM ones.
Just to make this clear, we are in this case: https://github.com/fox1t/modules-playground/blob/master/modules/cjs/ts.js#L17
"importing TS module into CJS context"

I can make a PR that uses a ".js bridge file" to get rid of that default prop.

@matt212
Copy link
Author

matt212 commented Dec 15, 2020

Hi @fox1t ,

You could go ahead and use below
created new feature branch fastify-passport
https://github.com/matt212/Nodejs_Postgresql_VanillaJS_Fastify/tree/fastify-passport
since In my platform I opted for custom strategies using JWT, hence the feature branch approach

@fox1t
Copy link
Member

fox1t commented Dec 15, 2020

Hi! My study project has esModuleInterop enabled, but there is the same import problem. I would like you to split imports (import Strategy from 'fastify-passport/Strategy') or make an import of the global object, for example: export default {passport, Strategy}. Airbnb does not recommend mixing imports: https://github.com/benmosher/eslint-plugin-import/blob/master/docs/rules/no-default-export.md

Speaking of this, here you are in the TS context with esModuleInterop: true, importing from a TS module: https://github.com/fox1t/modules-playground/blob/master/modules/ts-es-module-interop-true/ts.ts

This should just work!

@fox1t
Copy link
Member

fox1t commented Dec 15, 2020

Hi @fox1t ,

You could go ahead and use below
created new feature branch fastify-passport
https://github.com/matt212/Nodejs_Postgresql_VanillaJS_Fastify/tree/fastify-passport
since In my platform I opted for custom strategies using JWT, hence the feature branch approach

Can you try to import as const fastifyPassport= require('fastify-passport').default and let us know if it fixes this for you?

@matt212
Copy link
Author

matt212 commented Dec 15, 2020

Hi @fox1t
it seems to resolve the issue ,
image
Thanks for that @fox1t ,
Closing this issue

@matt212 matt212 closed this as completed Dec 15, 2020
matt212 added a commit to matt212/Nodejs_Postgresql_VanillaJS_Fastify that referenced this issue Dec 15, 2020
@fox1t
Copy link
Member

fox1t commented Dec 15, 2020

@Melodyn I've just seen that your file is a .js one, so you are not using any esModuleInterop: true since your context is a CJS context and your case is the same as @matt212 one. In fact, your fix is just to add .default.

@airhorns
Copy link
Member

That makes sense why it wasn't broken for some and broken for others, but, we should probably make it work by default for CJS contexts right?

@fox1t
Copy link
Member

fox1t commented Dec 15, 2020

Yes, there are 2 viable solutions:

  1. adding a "js bridge file" since TS explicitly doesn't support this functionality. (it leaves only .default prop if export default is used)
  2. adding namedExport and let the users use a named one in the contexts that don't support .default property (CJS and older Node.js versions mainly)

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

No branches or pull requests

4 participants