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

fix(router): properly parse json response in the FormAction #1605

Merged

Conversation

milosdanilov
Copy link
Contributor

@milosdanilov milosdanilov commented Feb 10, 2025

PR Checklist

Fixes the parsing of a JSON response created using the form server actions, used by the FormAction directive.

The problem arose because the json function, on the server, sets the Content-Type header to application/json; charset=utf-8 (see this line). The additional charset=utf-8 parameter caused the content-type check to fail, treating the response as plain text. As a result, the response was incorrectly parsed as text instead of JSON.

What is the new behavior?

This change ensures the response is correctly handled as JSON, even when the charset=utf-8 parameter is present in the Content-Type header.

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

The newsletter.page example in the analog-app is updated to show the email from the response, and additional cypress tests for newsletter page for this case are written.

Copy link

netlify bot commented Feb 10, 2025

Deploy Preview for analog-ng-app ready!

Name Link
🔨 Latest commit 62f94fc
🔍 Latest deploy log https://app.netlify.com/sites/analog-ng-app/deploys/67aba02fe9b1cd0007b72fac
😎 Deploy Preview https://deploy-preview-1605--analog-ng-app.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Feb 10, 2025

Deploy Preview for analog-blog ready!

Name Link
🔨 Latest commit 62f94fc
🔍 Latest deploy log https://app.netlify.com/sites/analog-blog/deploys/67aba02f9bab2d00086316c3
😎 Deploy Preview https://deploy-preview-1605--analog-blog.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Feb 10, 2025

Deploy Preview for analog-app ready!

Name Link
🔨 Latest commit 62f94fc
🔍 Latest deploy log https://app.netlify.com/sites/analog-app/deploys/67aba02f5b0c550008f930a5
😎 Deploy Preview https://deploy-preview-1605--analog-app.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Feb 10, 2025

Deploy Preview for analog-docs ready!

Name Link
🔨 Latest commit 62f94fc
🔍 Latest deploy log https://app.netlify.com/sites/analog-docs/deploys/67aba02f8985d10008c897a2
😎 Deploy Preview https://deploy-preview-1605--analog-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@brandonroberts
Copy link
Member

Thanks @milosdanilov, the fix looks good. We already have a FormAction example here that ca be updated. https://github.com/analogjs/analog/blob/beta/apps/analog-app/src/app/pages/newsletter.page.ts

Would you revert the changes to the cart and update that one instead?

@milosdanilov
Copy link
Contributor Author

@brandonroberts Sure, will do

@brandonroberts brandonroberts merged commit 6a8fc20 into analogjs:beta Feb 12, 2025
22 of 24 checks passed
@brandonroberts
Copy link
Member

Thanks! @allcontributors add @milosdanilov for code

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.

2 participants