Skip to content

Add the typeclass IsSendable #5

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

Merged
merged 7 commits into from
Feb 7, 2023
Merged

Conversation

gbagan
Copy link
Collaborator

@gbagan gbagan commented Jan 11, 2023

This refers to the issue: #2

Add the typeclass IsSendable whose instances are Boolean, Int, Number, Char, String, Array, Record.
Change the signature of postMessage with

postMessage :: forall msg. IsSendable msg => msg -> Worker -> Effect Unit

Prerequisites

  • Before opening a pull request, please check the DOM standard (https://dom.spec.whatwg.org/). If it doesn't appear in this spec, it may be present in the spec for one of the other purescript-web projects. Although MDN is a great resource, it is not a suitable reference for this project.

Description of the change

Clearly and concisely describe the purpose of the pull request. If this PR relates to an existing issue or change proposal, please link to it. Include any other background context that would help reviewers understand the motivation for this PR.


Checklist:

  • Added the change to the changelog's "Unreleased" section with a reference to this PR (e.g. "- Made a change (#0000)")
  • Linked any existing issues or proposals that this pull request should close
  • Updated or added relevant documentation
  • Added a test for the contribution (if applicable)

@gbagan gbagan requested a review from JordanMartinez January 11, 2023 22:08
@gbagan gbagan self-assigned this Jan 11, 2023
Copy link
Contributor

@JordanMartinez JordanMartinez left a comment

Choose a reason for hiding this comment

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

LGTM aside from the below things.

@gbagan
Copy link
Collaborator Author

gbagan commented Jan 19, 2023

@JordanMartinez
I have applied your suggestions.

@@ -21,8 +24,24 @@ instance IsSendable Char
instance IsSendable String
instance IsSendable a => IsSendable (Array a)
instance (RowToList r rl, IsSendableRowList rl) => IsSendable (Record r)
instance IsSendable Unit
instance IsSendable Void
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you clarify why you added an instance for Void? I'm not sure that's correct, but I'm not sure about that either.

@@ -21,8 +24,24 @@ instance IsSendable Char
instance IsSendable String
instance IsSendable a => IsSendable (Array a)
instance (RowToList r rl, IsSendableRowList rl) => IsSendable (Record r)
instance IsSendable Unit
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is there an instance for Unit? Can you clarify?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Actually, this instance and the instance for Void comes from the library workerbees but I can remove them if you want.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, ok. I think I missed that.

@JordanMartinez JordanMartinez merged commit 063ea75 into purescript-web:main Feb 7, 2023
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