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

Add the http-request-builder example #418

Merged
merged 3 commits into from
May 9, 2022

Conversation

mc1098
Copy link
Contributor

@mc1098 mc1098 commented May 7, 2022

This adds the same example as the http-request but using the builder
syntax.

Most of the code is the same and a fair degree of the change from macro
to builder syntax is trivial. Using Suspense with the builder syntax
does not seem to be covered in the documentation. This also shows off that structs
that derive Prop have public builders which may be required using the
builder syntax - such as with SuspenseProps.

I added a comment to the SuspenseProps part just because I don't think it is obvious
or documented (on the website) that by deriving Prop you get a free typed builder too :)

I know @lukechu10 spoke about changing the API around Suspense in the builder
syntax on Discord - so I'm aware you might have reason to not accept this PR yet (or at all) if
those changes are coming soon.

Closes #395

This adds the same example as the http-request but using the builder
syntax.

Most of the code is the same and a fair degree of the change from macro
to builder syntax is trivial. Using `Suspense` with the builder syntax
is not covered in the documentation. This also shows off that structs
that derive `Prop` have public builders which may be required using the
builder syntax - such as with `SuspenseProps`.
Copy link
Member

@lukechu10 lukechu10 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do in fact plan to change some details about the builder API but I'm happy to merge this PR now because it might be a while before I actually do anything. Just one small comment, could you please add an entry to examples/README.md describing the example? Otherwise, LGTM

@codecov
Copy link

codecov bot commented May 8, 2022

Codecov Report

Merging #418 (48faa9a) into master (c9e7623) will not change coverage.
The diff coverage is n/a.

❗ Current head 48faa9a differs from pull request most recent head 6962c1a. Consider uploading reports for the commit 6962c1a to get more accurate results

@@           Coverage Diff           @@
##           master     #418   +/-   ##
=======================================
  Coverage   64.48%   64.48%           
=======================================
  Files          49       49           
  Lines        7958     7958           
=======================================
  Hits         5132     5132           
  Misses       2826     2826           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c9e7623...6962c1a. Read the comment docs.

@mc1098
Copy link
Contributor Author

mc1098 commented May 8, 2022

could you please add an entry to examples/README.md describing the example?

Oh forgot to do that :)
Done!

Make title more consistent with other examples
@lukechu10
Copy link
Member

Thanks a lot. I just noticed a small nit in the index.html title but I changed it for you since I didn't want to bother you with such a small change.

@lukechu10 lukechu10 merged commit 49135bd into sycamore-rs:master May 9, 2022
@mc1098 mc1098 deleted the example-http-request-builder branch May 9, 2022 11:56
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.

http-request example with builder syntax
2 participants