Skip to content

export integrated cookie functionality#263

Merged
mcollina merged 5 commits intomasterfrom
export-integrated-cookie
Nov 25, 2023
Merged

export integrated cookie functionality#263
mcollina merged 5 commits intomasterfrom
export-integrated-cookie

Conversation

@Uzlopak
Copy link
Copy Markdown
Contributor

@Uzlopak Uzlopak commented Nov 14, 2023

This way we can use it in fastify/secure-session and co instead of the cookie package.

Checklist

Copy link
Copy Markdown
Member

@gurgunday gurgunday left a comment

Choose a reason for hiding this comment

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

Great, thanks

Comment thread cookie.js
Comment thread plugin.js Outdated
@Uzlopak
Copy link
Copy Markdown
Contributor Author

Uzlopak commented Nov 15, 2023

@mcollina PTAL

@mcollina
Copy link
Copy Markdown
Member

Plase require the entrypoint file in

const cookie = require('../cookie')
.

Copy link
Copy Markdown
Member

@jsumners jsumners left a comment

Choose a reason for hiding this comment

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

If the other module is going to use @fastify/cookie to manage cookies, why do we need to export the underlying tooling?

@Uzlopak
Copy link
Copy Markdown
Contributor Author

Uzlopak commented Nov 20, 2023

@mcollina
PTAL

Copy link
Copy Markdown
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

Can you document it in the README?

@gurgunday gurgunday requested a review from mcollina November 24, 2023 14:49
Comment thread README.md Outdated
you should register it prior to any other `onRequest` hooks that will depend
upon this plugin's actions.

It is also possible to import the low-level cookie parsing and serialization functions.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe add a link here to the header section regarding the import?

@gurgunday gurgunday requested a review from Fdawgs November 25, 2023 13:57
@Uzlopak
Copy link
Copy Markdown
Contributor Author

Uzlopak commented Nov 25, 2023

@mcollina PTAL

@gurgunday
Thank you for your work.

Copy link
Copy Markdown
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

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

Successfully merging this pull request may close these issues.

5 participants