Add order confirmation chapter #9

feihong · 2023-12-01T03:40:56Z

This chapter mostly introduces variant types but touches on a lot of other things like mel.raw, fun, arrays, and runtime_deps

jchavarri

phew! Amazing work, this chapter is quite extensive! I really like it.

I left a lot of minor suggestions, the most relevant one is prob the JSX and React.element relationship and the key attrs.

jchavarri · 2023-12-04T07:44:50Z

docs/order-confirmation/index.md

+The famed restauranteur Madame Jellobutter has opened a hot new pop-up
+restaurant called Emoji Cafe, and you've been commissioned to build the order
+confirmation widget on its website. Feeling adventurous, you decide to build it
+using Melange.


Love this intro 👩‍🍳

docs/order-confirmation/index.md

jchavarri · 2023-12-04T07:49:17Z

docs/order-confirmation/index.md

+
+Your OCaml code wouldn't compile if you just added a new constructor to
+`Item.t` without also updating `Item.toPrice`. Adding or removing constructors
+from a variant usually forces you to change relevant parts of your code.


"usually"? Sounds like the compiler is not precise sometimes 😄

Got rid of "usually" in 2d781b2. Also in the commit: added a new section about wildcard in switch expression to explain the case when adding or removing constructors wouldn't force you to change your code.

jchavarri · 2023-12-04T07:50:02Z

docs/order-confirmation/index.md

+
+## `fun` sugar syntax
+
+Incidentally, there's a sugar syntax for functions whose entire body is a switch


Why "Incidentally"? Maybe can be removed? I am also not sure about using "syntax sugar", I remember when I started using BuckleScript and Reason that this term would be thrown around, and I had no clue what it meant for a while 😅 i think it's a term mostly people familiar with parsers and programming langs might be familiar with, but maybe can be written in a different way:

Suggested change

Incidentally, there's a sugar syntax for functions whose entire body is a switch

There's an alternate, shorter way to write functions whose entire body is a `switch`

Stopped using the term syntax sugar in 895b759

docs/order-confirmation/index.md

jchavarri · 2023-12-04T08:33:33Z

docs/order-confirmation/index.md

+
+```
+ERROR in ./_build/default/src/order-confirmation/output/src/order-confirmation/Item.js 5:0-6:1
+Module not found: Error: Can't resolve './item.css' in '/home/fhsu/work/melange-for-react-devs/_build/default/src/order-confirmation/output/src/order-confirmation'


Suggested change

Module not found: Error: Can't resolve './item.css' in '/home/fhsu/work/melange-for-react-devs/_build/default/src/order-confirmation/output/src/order-confirmation'

Module not found: Error: Can't resolve './item.css' in '~/melange-for-react-devs/_build/default/src/order-confirmation/output/src/order-confirmation'

This part was removed from this PR, will reappear in next PR

docs/order-confirmation/index.md

jchavarri · 2023-12-04T08:39:40Z

docs/order-confirmation/index.md

+[^4]: Madame Jellobutter was passing by and just happened to catch a glimpse of
+  the unstyled component over your shoulder and puked in her mouth a little.


feihong

With the latest edits I made, I increased the size of the chapter and now it's almost two times as long as some other chapters.

I'm going to move this PR back to draft and cut out the styling with CSS parts to put them in the next chapter.

Update: Cut out the styling bits in 3a7d0a0

docs/order-confirmation/index.md

feihong · 2023-12-05T17:17:39Z

docs/order-confirmation/index.md

+
+Your OCaml code wouldn't compile if you just added a new constructor to
+`Item.t` without also updating `Item.toPrice`. Adding or removing constructors
+from a variant usually forces you to change relevant parts of your code.


Got rid of "usually" in 2d781b2. Also in the commit: added a new section about wildcard in switch expression to explain the case when adding or removing constructors wouldn't force you to change your code.

feihong · 2023-12-05T17:23:37Z

docs/order-confirmation/index.md

+
+## `fun` sugar syntax
+
+Incidentally, there's a sugar syntax for functions whose entire body is a switch


Stopped using the term syntax sugar in 895b759

feihong · 2023-12-05T17:23:51Z

docs/order-confirmation/index.md

+<<< Snippets.re#to-emoji
+
+Using the `fun` syntax is completely equivalent to using a switch expression,
+so it's up to your personal taste whether you want to use this sugar syntax.


Fixed in 895b759

feihong · 2023-12-06T02:19:46Z

docs/order-confirmation/index.md

+  This means the `Order` component has a single prop named `items`.
+- We sum up the prices of all items using
+  [Js.Array.reduce](https://melange.re/v2.1.0/api/re/melange/Js/Array/index.html#val-reduce).
+  This is identical to JavaScript's [Array.reduce


Fixed in 68a424d

feihong · 2023-12-06T02:19:58Z

docs/order-confirmation/index.md

+  except that the Melange version requires the initial value to be passed in.
+- For each order, we render an `Item` component via
+  [Js.Array.map](https://melange.re/v2.1.0/api/re/melange/Js/Array/index.html#val-map),
+  which is equivalent to the [Array.map


Fixed in 68a424d

feihong · 2023-12-06T03:53:46Z

docs/order-confirmation/index.md

+  method](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/map).
+- The `React.array` function has the type signature `array(React.element) =>
+  React.element`, meaning it converts an array of `React.element`s to
+  `React.element`. All expressions in JSX must have the type `React.element`.


I tried to tackle this by adding an entire section for this instead of relegating it to a single bullet point: c85c07a

jchavarri

Looks great! 🚀 👩‍🍳

Just a thought, prob too late already, but did you consider using lists instead of arrays across the board? Would that make anything easier? As we don't need random access, and probably new items in the order are just appended, wonder if it'd have been a better data structure 🤔

jchavarri · 2023-12-07T09:22:01Z

docs/order-confirmation/Snippets.re

+      <td>
+        {item
+         |> toPrice
+         |> Js.Float.toFixedWithPrecision(~digits=2)


Heads up: these functions are being modified with Melange v3: melange-re/melange#673

No action needed, just once v3 is released we'll have to remember to update the book so that all snippets compile.

jchavarri · 2023-12-07T09:28:02Z

docs/order-confirmation/Snippets.re

+  // #region to-emoji
+  let toEmoji =
+    fun
+    | Sandwich => {js|🥪|js}


I just realized we could mention that {js||js} is Melange-specific, and it's not available in native OCaml.

Maybe we could come up with some kind of tips, I noticed vitepress supports them:

::: tip This is a tip. :::

Added a tip in the chapter where we first covered {js||js} strings: d4aa3b7

jchavarri · 2023-12-07T09:32:24Z

docs/order-confirmation/Snippets.re

+      items
+      |> Js.Array.reduce((acc, order) => acc +. Item.toPrice(order), 0.);
+
+    let itemRows = items |> Js.Array.map(item => <Item item />);


Would it make sense to add the explicit annotation here? As I read the text that uses this code, I noticed it's talking about the types themselves:

melange-for-react-devs/docs/order-confirmation/index.md

Lines 167 to 168 in ed2005d

To better see what types are at play, it might make sense to refactor

`Order.make` like so:

Added type annotation in 6634e59 and also added an explanation that React.array doesn't change the underlying JS object.

jchavarri · 2023-12-07T09:33:56Z

docs/order-confirmation/index.md

+In practice, it's common for Melange bindings do not match one-to-one to their
+JavaScript counterparts.


This sentence is strangely constructed, maybe this?

Suggested change

In practice, it's common for Melange bindings do not match one-to-one to their

JavaScript counterparts.

In practice, it's common for Melange bindings to not match one-to-one their

JavaScript counterparts.

The sentence is also a bit misleading. The JavaScript API includes the index as well. Maybe something like:

It's common for Melange bindings to represent JavaScript APIs with optional arguments using multiple OCaml externals.

Can link to this section: https://melange.re/v2.1.0/communicate-with-javascript/#approach-1-multiple-external-functions

Added the link and tried to make the statement more specific in 132bda1

jchavarri · 2023-12-07T09:41:11Z

docs/order-confirmation/index.md

+
+<<< Snippets.re#make
+
+The `make` function has a single labeled argument, `~item`, of type `t`. This


Would this make sense? A bit redundant, but might help clarifying:

Suggested change

The `make` function has a single labeled argument, `~item`, of type `t`. This

The `make` function has a single labeled argument, `~item`, of type `Item.t`. This

Changed from t to Item.t in 1176139

feihong

Just a thought, prob too late already, but did you consider using lists instead of arrays across the board?

Yes, in a future chapter, I'll migrate from arrays to lists. I figured I would start with arrays since that's what most frontend devs are more familiar with.

feihong · 2023-12-08T02:40:22Z

docs/order-confirmation/Snippets.re

+  // #region to-emoji
+  let toEmoji =
+    fun
+    | Sandwich => {js|🥪|js}


Added a tip in the chapter where we first covered {js||js} strings: d4aa3b7

feihong · 2023-12-08T03:07:41Z

docs/order-confirmation/Snippets.re

+      items
+      |> Js.Array.reduce((acc, order) => acc +. Item.toPrice(order), 0.);
+
+    let itemRows = items |> Js.Array.map(item => <Item item />);


Added type annotation in 6634e59 and also added an explanation that React.array doesn't change the underlying JS object.

feihong · 2023-12-08T03:22:42Z

docs/order-confirmation/index.md

+In practice, it's common for Melange bindings do not match one-to-one to their
+JavaScript counterparts.


Added the link and tried to make the statement more specific in 132bda1

feihong · 2023-12-08T03:25:07Z

docs/order-confirmation/index.md

+
+<<< Snippets.re#make
+
+The `make` function has a single labeled argument, `~item`, of type `t`. This


Changed from t to Item.t in 1176139

feihong requested a review from jchavarri December 1, 2023 03:48

feihong marked this pull request as ready for review December 1, 2023 03:49

Add order confirmation chapter

45f81fe

feihong force-pushed the order-conf branch from 2c16e79 to 45f81fe Compare December 1, 2023 16:11

jchavarri reviewed Dec 4, 2023

View reviewed changes

feihong added 4 commits December 5, 2023 11:13

Revise variant type section, add wildcard section

2d781b2

Stop referring to fun as sugar syntax

895b759

Small fixes to Order component section

68a424d

Add a section dedicated to explaining React.array

c85c07a

feihong commented Dec 6, 2023

View reviewed changes

feihong marked this pull request as draft December 6, 2023 03:56

feihong added 2 commits December 6, 2023 10:42

Delete sections on styling with CSS

3a7d0a0

Add Js.Array.mapi section along with many small fixes

8e4c39f

feihong marked this pull request as ready for review December 6, 2023 18:03

feihong requested a review from jchavarri December 6, 2023 18:03

Remove accidental occurrences of className

ed2005d

jchavarri approved these changes Dec 7, 2023

View reviewed changes

feihong added 4 commits December 7, 2023 20:37

Add tip to say that {js||js} strings aren't available in native OCaml

d4aa3b7

Add explicit type annotation in order-make-item-rows snippet

6634e59

Sharpen the statement about bindings

132bda1

Change reference to prop type from t to Item.t

1176139

feihong commented Dec 8, 2023

View reviewed changes

feihong merged commit 7543861 into main Dec 8, 2023

feihong deleted the order-conf branch December 8, 2023 16:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add order confirmation chapter #9

Add order confirmation chapter #9

feihong commented Dec 1, 2023 •

edited

Loading

jchavarri left a comment

jchavarri Dec 4, 2023

jchavarri Dec 4, 2023

feihong Dec 5, 2023

jchavarri Dec 4, 2023

feihong Dec 5, 2023

jchavarri Dec 4, 2023

feihong Dec 6, 2023

jchavarri Dec 4, 2023

feihong left a comment •

edited

Loading

feihong Dec 5, 2023

feihong Dec 5, 2023

feihong Dec 5, 2023

feihong Dec 6, 2023

feihong Dec 6, 2023

feihong Dec 6, 2023

jchavarri left a comment

jchavarri Dec 7, 2023

jchavarri Dec 7, 2023

feihong Dec 8, 2023

jchavarri Dec 7, 2023

feihong Dec 8, 2023

jchavarri Dec 7, 2023

jchavarri Dec 7, 2023

feihong Dec 8, 2023

jchavarri Dec 7, 2023

feihong Dec 8, 2023

feihong left a comment

feihong Dec 8, 2023

feihong Dec 8, 2023

feihong Dec 8, 2023

feihong Dec 8, 2023


		## `fun` sugar syntax

		Incidentally, there's a sugar syntax for functions whose entire body is a switch

	Incidentally, there's a sugar syntax for functions whose entire body is a switch
	There's an alternate, shorter way to write functions whose entire body is a `switch`

	Module not found: Error: Can't resolve './item.css' in '/home/fhsu/work/melange-for-react-devs/_build/default/src/order-confirmation/output/src/order-confirmation'
	Module not found: Error: Can't resolve './item.css' in '~/melange-for-react-devs/_build/default/src/order-confirmation/output/src/order-confirmation'

		[^4]: Madame Jellobutter was passing by and just happened to catch a glimpse of
		the unstyled component over your shoulder and puked in her mouth a little.

	To better see what types are at play, it might make sense to refactor
	`Order.make` like so:

		In practice, it's common for Melange bindings do not match one-to-one to their
		JavaScript counterparts.


		<<< Snippets.re#make

		The `make` function has a single labeled argument, `~item`, of type `t`. This

Add order confirmation chapter #9

Add order confirmation chapter #9

Conversation

feihong commented Dec 1, 2023 • edited Loading

jchavarri left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

feihong left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jchavarri left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

feihong left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

feihong commented Dec 1, 2023 •

edited

Loading

feihong left a comment •

edited

Loading