feat: add cors utils #322
feat: add cors utils #322
Conversation
|
Thanks for PR dear @NozomuIkuta. Can we please directly inline the logic in h3 package? This way we can directly test and release it. |
README.md
Outdated
| @@ -157,6 +157,7 @@ H3 has a concept of composable utilities that accept `event` (from `eventHandler | |||
| - `getSession(event, { password, name?, cookie?, seal?, crypto? })` | |||
| - `updateSession(event, { password, name?, cookie?, seal?, crypto? }), update)` | |||
| - `clearSession(event, { password, name?, cookie?, seal?, crypto? }))` | |||
| - `handleCors(options)` (see [h3-cors](https://github.com/NozomuIkuta/h3-cors) for more detail) | |||
There was a problem hiding this comment.
Let me consider it in another time how we can organize README, because it needs lots of words to explain CORS features. Or, we might want to create a Docus website for h3.
There was a problem hiding this comment.
Indeed. If possible adding help in JSDocs would be the best. In the future we should generate README baased on JSDocs and add more. Link is good for me until then 👍🏼
Codecov Report
@@ Coverage Diff @@
## main #322 +/- ##
==========================================
+ Coverage 71.92% 74.32% +2.39%
==========================================
Files 23 26 +3
Lines 2066 2294 +228
Branches 303 354 +51
==========================================
+ Hits 1486 1705 +219
- Misses 580 589 +9
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
src/utils/cors/utils.ts
Outdated
| return defu(options, defaultOptions); | ||
| } | ||
|
|
||
| export function isPreflight(event: H3Event): boolean { |
src/utils/cors/types.ts
Outdated
| } | ||
| | EmptyHeader; | ||
|
|
||
| export type AccessControlMaxAgeHeader = |
There was a problem hiding this comment.
Since we are exposing this types to end user, prefix would be better. (Do we really need to expose all individuals?)
There was a problem hiding this comment.
Fixed: 052c215
I modified exports and make only CorsOptions public, which is useful to restrict argument type for CORS utilities (e.g. handleCors(event, corsOptions)).
|
It's worth merging content of NozomuIkuta/h3-cors#8 into this PR. |
|
#322 (comment) is fixed: bb7af39. |
|
I applied your feedback and the remaining PR (NozomuIkuta/h3-cors#8). |
src/utils/cors/handler.ts
Outdated
| preflight: { statusCode }, | ||
| } = resolveCorsOptions(options); | ||
|
|
||
| return defineEventHandler((event) => { |
There was a problem hiding this comment.
handle shouldn't create a wrapper handler but directly accept event to append required headers.
|
I have pushed couple of refactors. Really nice stuff thanks for helping to bring it to the core 👍🏼 |
Resolves #82
Related: NozomuIkuta/h3-cors#10
This PR is created according to @pi0's comment to my package.
I have upgraded h3 version in my package (and improved code coverage to 100% 👍).
Now, please review this PR and check whether I'm doing as you intended.