Skip to content
This repository has been archived by the owner on Sep 8, 2021. It is now read-only.

Option #12

Merged
merged 1 commit into from
Jun 30, 2020
Merged

Option #12

merged 1 commit into from
Jun 30, 2020

Conversation

JamieB-gu
Copy link
Contributor

What does this change?

Added an Option type and companion functions. This is a clone of the type that we've been using for some time in apps-rendering. I'm migrating it to this repo because I'd like to make use of it in liveblog-rendering as well, and it may be useful for DCR if and when it starts to consume liveblog-rendering.

Changes

  • New Option type, consisting of Some and None
  • fromNullable constructor to create from JavaScript's two "nullable" types, null and undefined
  • some and none convenience functions to create an Option
  • withDefault, map and andThen functions for working with Option

Copy link

@alexduf alexduf left a comment

Choose a reason for hiding this comment

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

The code looks good, but should we really impose functional style to the rest of the rendering teams?

@JamieB-gu JamieB-gu merged commit d2c4bc4 into master Jun 30, 2020
@JamieB-gu JamieB-gu deleted the option branch June 30, 2020 15:35
@JamieB-gu
Copy link
Contributor Author

should we really impose functional style to the rest of the rendering teams

Good question. I'm doing this for now because I need these changes for the work on liveblog-rendering. If it turns out that the style doesn't work for us I'm happy to refactor.

Two other things occur to me:

  1. apps-rendering is currently the only platform consuming guardian/types, and it already has this code available
  2. The nice thing about migrating to this from classes is that anyone using it doesn't have to use any of the companion functions. Both Option and Result are plain JS objects, and examples of TS discriminated unions, so you can work with them while completely ignoring map, withDefault etc.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants