qwik
qwik copied to clipboard
feat: First Class Cookie Support
What is it?
- [x] Feature / enhancement
- [ ] Bug
- [ ] Docs / tests
Description
The current headers implementation concatenates values with the same header using ", ". This works for all headers except Set-Cookie where commas are not allowed.
Other frameworks appear to rely on node-fetch having a raw() function that returns the non-concatenated header values. This seems like a poor assumption as we should not rely on a third party package being available inside various edge environments.
This PR creates a Cookie class that is accessible on the RequestContext. It allows a type-safe API for getting/setting cookies and allows us to dodge the header concatenation issue entirely.
Inspired by this Astro RFC: https://github.com/withastro/astro/pull/4876
Fixes: #1851
Checklist:
- [x] My code follows the developer guidelines of this project
- [x] I have performed a self-review of my own code
- [x] I have made corresponding changes to the documentation
- [x] Added new tests to cover the fix / functionality
Run & review this pull request in StackBlitz Codeflow.
I think you can't use ', '
here
Set-Cookie: id=a3fWa; Expires=Wed, 21 Oct 2015 07:28:00 GMT
The <cookie-name>
and <cookie-value>
can't contains whitespace and comma but the Expires
attribute can.
Great catch, @langbamit! I think Svelte is using a special parser. I'll look into a better solution. Thanks!
Nois, this is exciting!! ") Pls don't hate me for this question, but - any estimate if the could be a merge in near future? smiley
Definitely the near future. :+1:
Hey @manucorporat. I'm unable to run yarn api.update locally after rebasing because of this error:
packages/qwik/src/optimizer/src/plugins/vite.ts:252:11 - error TS2322: Type '{ outDir: string; rollupOptions: { input: string[]; preserveEntrySignatures: "exports-only"; output: OutputOptions; treeshake: { moduleSideEffects: false; }; onwarn: (warning: RollupWarning, warn: WarningHandler) => void; }; modulePreload: { ...; }; dynamicImportVarsOptions: { ...; }; }' is not assignable to type 'BuildOptions'.
Object literal may only specify known properties, and 'modulePreload' does not exist in type 'BuildOptions'.
252 modulePreload: {
~~~~~~~~~~~~~~~~
253 polyfill: false,
~~~~~~~~~~~~~~~~~~~~~~~~~~~~
254 },
~~~~~~~~~~~
node_modules/vite/dist/node/index.d.ts:2448:5
2448 build?: BuildOptions;
~~~~~
The expected type comes from property 'build' which is declared here on type 'UserConfig'
Found 1 error in packages/qwik/src/optimizer/src/plugins/vite.ts:252
Any ideas? I could revert the change where I removed the cookies field from the Netlify Context and unblock this for merging.
Edit: Nevermind. Found a workaround. I'm leaving the error message so you're aware of the type error. :+1:
So awesome, great PR! Thanks @nnelgxorz!!