Split current 'order with promo' chapter into two separate chapters #52

feihong · 2024-09-11T02:39:45Z

Most of the content is the same, just split up into two chapters. However, I did add a few extra sections and exercises.

Notably, I removed all the Effects material and will use it for a different application example.

Save it for next chapter

jchavarri

I finished a review of the first of the two chapters 😌 Submitting because there were a few comments already, I will go with the other one tomorrow.

docs/poly-to-normal-variant/index.md

jchavarri · 2024-09-11T13:30:21Z

docs/poly-to-normal-variant/index.md

+implement it with a normal variant instead of a polymorphic variant. This small
+refactor should give us more insight into OCaml's type system. Additionally,
+normal variants are better than polymorphic variants at "documenting" the types
+that will be used in your program, since they must always be explicitly defined


It's nice to mention documentation, but this doesn't explain much about the technical use cases where normal variants are more useful than poly variants:

Errors and signatures are simpler

Stronger type guarantees (pattern match "completeness", safer refactoring)

Potentially better performance (both at build time and runtime)

The downsides are that:

Normal variants can be "expanded" dynamically, so for cases where it's unclear if more cases might be needed (or where the author of the types don't control their usage, like library authors) polyvars might fit better because extra flexibility

They always need to be namespaced with module or annotated explicitly when used, while polyvars values can just be written without any annotations

Do you think it's worth mentioning these aspects?

Yes, I think it's worthwhile to go over these points. I prefer to bring them up one by one and then summarize at the end, but we'll see how it goes 🤞

Errors and signatures are simpler

Here, do you mean that error messages for normal variants are simpler?

Normal variants can be "expanded" dynamically, so for cases where it's unclear if more cases might be needed

Do you mean that normal variants cannot be expanded dynamically?

Here, do you mean that error messages for normal variants are simpler?

Yes.

Do you mean that normal variants cannot be expanded dynamically?

Yes, I said it the other way around 🙃

docs/poly-to-normal-variant/index.md

jchavarri · 2024-09-11T13:46:25Z

docs/poly-to-normal-variant/index.md

+
+<<< Types.re#add-type-arg{1}
+
+This is somewhat like accidentally using a variable in a function but forgetting


Something that helps me wrap my head around these arguments is that they will be used by the compiler to "carry" the information about which extra tags are being added to the open variant, on each usage.

docs/poly-to-normal-variant/index.md

jchavarri · 2024-09-11T13:51:34Z

docs/poly-to-normal-variant/index.md

+In the next version of Melange, polymorphic variant definitions no longer
+require a space between `[` and `|`.


Wait :wat: is this because of some changes in the OCaml compiler, or Reason? In any case, I don't think this is Melange responsibility, so I would not add this warning unless it refers to the last package and version that didn't support it and the first one that will.

I guess it was a Reason parsing bug, see reasonml/reason#2781. Not sure if that fix is in the current melange 4.0.1 release.

Then we shouldn't refer to Melange new version. Reason is independent package than Melange, and they can be combined in different ways. In other words, having Melange 4.0.1 doesn't involve getting the latest reason.

feihong · 2024-09-11T03:06:33Z

docs/poly-to-normal-variant/index.md

+
+::: warning
+
+In the next version of Melange, polymorphic variant definitions no longer


Does the current version already fix this issue? By that I mean 4.0.1-52.

See #52 (comment). Melange version is independent from Reason version.

feihong · 2024-09-11T03:07:23Z

docs/poly-to-normal-variant/index.md

+
+:::
+
+<b>4.</b> When you enter a promo code that is blank (empty or consists of only


This is a new exercise. It doesn't have much to do with types, but it allows me to not put too many exercises in the next chapter.

feihong · 2024-09-11T14:30:32Z

docs/poly-to-normal-variant/index.md

+In the next version of Melange, polymorphic variant definitions no longer
+require a space between `[` and `|`.


I guess it was a Reason parsing bug, see reasonml/reason#2781. Not sure if that fix is in the current melange 4.0.1 release.

feihong

Made a number of fixes but still thinking about how to work in "pros and cons" from https://github.com/melange-re/melange-for-react-devs/pull/52/files#r1754546821

docs/poly-to-normal-variant/index.md

jchavarri

Looks great! It's still unclear to me how the convo about technical merits of polyvar vs normal variants will be resolved?

jchavarri · 2024-09-13T06:02:34Z

docs/order-with-promo/index.md

 ## Add `Promo` to `Order`

 Add the `Promo` component to the `Order` component:

-<<< Order.re#make{3,22-25,28}
+<<< Order.re#make{3,5,18-25,28}

 A breakdown:

 - Create a new state variable called `discount` (along with its attendant


I've never read "attendant" in this sense.

jchavarri · 2024-09-13T07:52:09Z

docs/poly-to-normal-variant/index.md

+
+::: warning
+
+In the next version of [Reason](https://reasonml.github.io/), polymorphic


Can be tested here. Note that refmt still adds the space.

Re: "the next version", how do you plan on tracking a reminder or similar so that this chapter is updated when the next release is out?

… type inside a submodule

feihong added 9 commits August 6, 2024 17:50

Add chapter 'from polymorphic to normal variant'

b7f76c0

Clarify difference between variant constructor and variant tag

110270e

Delete type material from 'order with promo' chapter

b744821

Add exercise 4 in 'from polymorphic to normal variant' chapter

de44437

Remove effects material

7821980

Update the code in src/order-with-promo

b149b82

Complete exercise 2

1d10bc7

Add exercise 4

f5f2074

Delete exercise 5

d00e9ab

Save it for next chapter

feihong force-pushed the poly-to-normal branch from ebd68ea to d00e9ab Compare September 11, 2024 03:11

jchavarri reviewed Sep 11, 2024

View reviewed changes

feihong marked this pull request as ready for review September 11, 2024 14:21

feihong commented Sep 11, 2024

View reviewed changes

feihong added 2 commits September 11, 2024 20:52

Fix typos

42b2f93

Reword > section for clarity

6354066

feihong commented Sep 12, 2024

View reviewed changes

Ascribe change in polymorphic variant syntax to Reason, not Melange

5fe39cf

jchavarri approved these changes Sep 13, 2024

View reviewed changes

feihong mentioned this pull request Sep 17, 2024

Delete the trollish exercise in 'From Polymorphic to Normal Variant' chapter #54

Open

Add sections about type-related error messages and putting discount…

74fa01c

… type inside a submodule

feihong mentioned this pull request Sep 25, 2024

Add pros and cons of polymorphic variants versus normal variants #56

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Split current 'order with promo' chapter into two separate chapters #52

Split current 'order with promo' chapter into two separate chapters #52

feihong commented Sep 11, 2024 •

edited

Loading

jchavarri left a comment

jchavarri Sep 11, 2024

feihong Sep 12, 2024

feihong Sep 12, 2024 •

edited

Loading

jchavarri Sep 12, 2024

jchavarri Sep 11, 2024

jchavarri Sep 11, 2024

feihong Sep 11, 2024

jchavarri Sep 12, 2024

feihong Sep 11, 2024 •

edited

Loading

jchavarri Sep 12, 2024

feihong Sep 11, 2024 •

edited

Loading

feihong Sep 11, 2024

feihong left a comment

jchavarri left a comment

jchavarri Sep 13, 2024

jchavarri Sep 13, 2024


		<<< Types.re#add-type-arg{1}

		This is somewhat like accidentally using a variable in a function but forgetting

		In the next version of Melange, polymorphic variant definitions no longer
		require a space between `[` and `\|`.


		::: warning

		In the next version of Melange, polymorphic variant definitions no longer


		:::

		<b>4.</b> When you enter a promo code that is blank (empty or consists of only


		::: warning

		In the next version of [Reason](https://reasonml.github.io/), polymorphic

Split current 'order with promo' chapter into two separate chapters #52

Are you sure you want to change the base?

Split current 'order with promo' chapter into two separate chapters #52

Conversation

feihong commented Sep 11, 2024 • 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

feihong Sep 12, 2024 • 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

feihong Sep 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

feihong Sep 11, 2024 • edited Loading

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

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

feihong commented Sep 11, 2024 •

edited

Loading

feihong Sep 12, 2024 •

edited

Loading

feihong Sep 11, 2024 •

edited

Loading

feihong Sep 11, 2024 •

edited

Loading