feat: Issue #3336148: Add support for forms #247
Conversation
|
|
||
| const { data: page, error } = await useCeApi(path, useFetchOptions, true) | ||
| if (import.meta.server) { | ||
| serverResponse.value = useRequestEvent(nuxtApp).context.nitro.response |
There was a problem hiding this comment.
I'm wondering whether we need to have the separate serverResponse state really. Let's try whether it works without as well. It seems we could directly set page.value / pageError here, not?
page should be already in a state, so it should be correctly hydrated on client-side with that state then, no?
There was a problem hiding this comment.
it is needed to share the server data to the client, its not being hydrated
to make this better I added a line so it's cleared after its used
| passThroughHeaders(nuxtApp, serverResponse.value.headers) | ||
| // Clear the server response state after it was sent to the client. | ||
| if (import.meta.client) { | ||
| serverResponse.value = null |
There was a problem hiding this comment.
I'm still not 100% convinced we need serverResponse for hydrating things, since page should be already hydrated. But let's merge and re-check things once we have it merged and working.
There was a problem hiding this comment.
Tested and ensured it's not double-hydrated. maybe code could be nicer, but that seems all good!
|
|
||
| // Check if the page data is already provided, e.g. by a form response. | ||
| if (serverResponse.value && serverResponse.value._data) { | ||
| page.value = serverResponse.value._data |
There was a problem hiding this comment.
this never sets pageErrror like the other if part. does it actually handle error codes correctly?
playground/middleware/formHandler.ts
Outdated
| @@ -0,0 +1,34 @@ | |||
| import { readFormData, createError } from 'h3' | |||
There was a problem hiding this comment.
let's rename this file to "drupalFormHandler" to clarify it's only about the drupal-form, not every form.
Also I'm wondering whether we can declare the middleware in a way it is NOT shipped in the client bundle? This codes never ever needs to run on the client, so it seems to be silly to have it there. Would using server-middleware instead of route middleware be an option that helps us there?
https://nuxt.com/docs/guide/directory-structure/server#server-middleware
There was a problem hiding this comment.
I've moved it to server middleware which is global(cannot be imported in page, which should be fine)
but also as a page middleware, when using import.meta.server the code doesn't leak into client bundle
playground/middleware/formHandler.ts
Outdated
| throw createError({ | ||
| statusCode: 400, | ||
| statusMessage: 'Bad Request', | ||
| message: 'The request contains invalid form data or no form data at all.', |
There was a problem hiding this comment.
Let's say
"No or invalid form data given. See drupalFormHandler."
| headers: Object.fromEntries(response.headers.entries()), | ||
| } | ||
| return | ||
| } |
There was a problem hiding this comment.
what about the else case here. seems to be an error case which is silently ignored. we should error out properly also
There was a problem hiding this comment.
it isn't an error but it's unnecessary so i'll remove it
| @@ -0,0 +1,39 @@ | |||
| export default defineEventHandler(async (event) => { | |||
There was a problem hiding this comment.
I don't think that customizing this handler is a use-case, however we might want to update it within the module. Thus imo this should ship with the module, not be copied as part of the skeleton.
Let's add it via https://nuxt.com/docs/api/kit/nitro#addserverhandler - that has a middleware option
vloss3
force-pushed
the
feature/LDP-2492
branch
from
4372680 to
7ab5436
Compare
* feat: Issue #3336148: Add support for forms * LDP-2492: Remove unnecessary if * LDP-2492: Improve redirect code * LDP-2492: Improve internal redirect code * LDP-2492: Revert redirect code * LDP-2492: Improve code and use page middleware * LDP-2492: Add server middleware and error handling * LDP-2492: Move middleware to module * LDP-2492: Include error message commit * LDP-2492: Add imports
…#256) * improve: LDP-2578: added getPageLayout helper in useDrupalCe composable * minor changes * chore: Bump micromatch from 4.0.5 to 4.0.8 (#254) Bumps [micromatch](https://github.com/micromatch/micromatch) from 4.0.5 to 4.0.8. - [Release notes](https://github.com/micromatch/micromatch/releases) - [Changelog](https://github.com/micromatch/micromatch/blob/4.0.8/CHANGELOG.md) - [Commits](micromatch/micromatch@4.0.5...4.0.8) --- updated-dependencies: - dependency-name: micromatch dependency-type: indirect ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * feat: Issue #3336148: Add support for drupal-forms (#247) * feat: Issue #3336148: Add support for forms * LDP-2492: Remove unnecessary if * LDP-2492: Improve redirect code * LDP-2492: Improve internal redirect code * LDP-2492: Revert redirect code * LDP-2492: Improve code and use page middleware * LDP-2492: Add server middleware and error handling * LDP-2492: Move middleware to module * LDP-2492: Include error message commit * LDP-2492: Add imports --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Alexandru Teodor <[email protected]>
No description provided.