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

Testing new iron-session package version #1156

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

fforres
Copy link

@fforres fforres commented Nov 6, 2024

Description

Related to #1130

Passes all tests, however finding errors on building the final package (using current projects typescript version). As iron-session built package specifies one of their types to be a generic.

node_modules/.pnpm/[email protected]/node_modules/iron-session/dist/index.d.cts:112:77 - error TS2315: Type 'ServerResponse' is not generic.

112     <T extends object>(req: http.IncomingMessage | Request, res: Response | http.ServerResponse<http.IncomingMessage>, sessionOptions: SessionOptions): Promise<IronSession<T>>;
                                                                                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

I did manually remove the offending type from the created file, built/packaged/released on my account: https://www.npmjs.com/package/@fforres/workos-inc-node and from my tests on a monorepo, seems to be working.

If this helps moving

Documentation

Does this require changes to the WorkOS Docs? E.g. the API Reference or code snippets need updates.

[ ] Yes

If yes, link a related docs PR and add a docs maintainer as a reviewer. Their approval is required.

@matheins
Copy link

@stanleyphu this would really unblock the cloudflare worker community

@fforres
Copy link
Author

fforres commented Nov 22, 2024

Been testing it for a couple of weeks now in a real application and no issues. Real fix should be different, but would help the many-many folks using workers 🙏🏼

Also pinging @mattgd @gcarvelli JIC?

@PaulAsjes
Copy link
Contributor

Thanks for the PR @fforres! Problem at the moment is that we need to support Node v16, which iron-session v8 does not if memory serves. We try to circumvent this by providing a worker specific entry point, but clearly something isn't working as expected.

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

Successfully merging this pull request may close these issues.

3 participants