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

[legacy-framework] Use cookie prefix from config.blitz.js instead of package.json name field #1976

Closed
wants to merge 8 commits into from

Conversation

piotrski
Copy link

Closes: blitz-js/legacy-framework#329, blitz-js/legacy-framework#335

What are the changes and their implications?

Adds new cookiePrefix property to sessionMiddleware config.

module.exports = {
  middleware: [
    sessionMiddleware({
+     cookiePrefix: 'cookie',
      isAuthorized: simpleRolesIsAuthorized,
    }),
  ],
}

Instead of using the name from your package.json as a session cookie prefix, it uses the cookiePrefix from blitz.config.js

Should we add information on this somewhere in the docs?

Checklist

  • Changes covered by tests (tests added if needed)
  • PR submitted to blitzjs.com for any user facing changes

@piotrski piotrski force-pushed the cookie-prefix-from-config branch from 43a000a to 9c75f8d Compare February 20, 2021 16:13
Copy link
Member

@flybayer flybayer left a comment

Choose a reason for hiding this comment

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

Awesome, great job!

Main change is use name instead of type because we might need type for something else later that actually indicates type.

Then please open a PR to the docs repo to update the docs for this change

packages/config/src/index.ts Outdated Show resolved Hide resolved
packages/core/src/blitz-data.tsx Outdated Show resolved Hide resolved
packages/generator/templates/app/blitz.config.js Outdated Show resolved Hide resolved
packages/server/src/supertokens.ts Outdated Show resolved Hide resolved
@flybayer
Copy link
Member

Hey @piotrski, are you able to finish this up?

@piotrski
Copy link
Author

Sure. Sorry I had a lot of stuff on my mid and was sure it was already done.

@flybayer
Copy link
Member

flybayer commented Mar 17, 2021

@piotrski ok no problem :) I should have said something earlier

@flybayer
Copy link
Member

flybayer commented May 5, 2021

@mabadir can you resolve the merge conflicts too?

@mabadir
Copy link
Collaborator

mabadir commented May 5, 2021

Yes, next on my list.

@mabadir
Copy link
Collaborator

mabadir commented May 6, 2021

The file packages/core/src/supertokens.ts was recently removed from canary. I see @piotrski; made a change to this file to include the cookiePrefix to the configuration.
I assume it is safe to remove this file from the PR, as it’s not used in the core package any longer.

@flybayer any thoughts?

@flybayer
Copy link
Member

flybayer commented May 6, 2021

@mabadir
packages/core/src/supertokens.ts is now https://github.com/blitz-js/blitz/blob/canary/packages/core/src/auth/auth-types.ts

and packages/server/src/supertokens.ts is now https://github.com/blitz-js/blitz/blob/canary/packages/core/src/server/auth/sessions.ts

@flybayer
Copy link
Member

flybayer commented May 6, 2021

@mabadir and then we also need a PR to the docs repo to update docs for this change

@mabadir mabadir closed this May 8, 2021
@itsdillon itsdillon changed the title Use cookie prefix from config.blitz.js instead of package.json name field [legacy-framework] Use cookie prefix from config.blitz.js instead of package.json name field Jul 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Change session cookie prefix to have an explict config instead of using package.json name field
4 participants