Skip to content

Conversation

@nojaf
Copy link
Member

@nojaf nojaf commented Jan 19, 2026

I would like to start creating some alphas of a v12+ world

@@ -0,0 +1,57 @@
// Generated by ReScript, PLEASE EDIT WITH CARE

import * as Primitive_option from "@rescript/runtime/lib/es6/Primitive_option.js";
Copy link
Member Author

Choose a reason for hiding this comment

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

This is why I added @rescript/runtime to the dependencies.
I think it is correct to do so.

Copy link
Member

Choose a reason for hiding this comment

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

Would making @rescript/runtime a peer dependency work?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure, the reason I want to be explicit with "@rescript/runtime" is that I see this popping up in isolated installs.

"sources": [{ "dir": "src", "subdirs": true }],
"package-specs": [{ "module": "commonjs", "in-source": true }],
"suffix": ".bs.js",
"bs-dev-dependencies": [],
Copy link
Member Author

Choose a reason for hiding this comment

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

rescript-lang/rescript#8196 brought me here

let safeMakeEvent = eventName =>
if Js.typeof(event) == "function" {
if typeof(event) == #function {
makeEventIE11Compatible(eventName)
Copy link
Member Author

Choose a reason for hiding this comment

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

I hope we no longer care about IE11?

Copy link
Member

@mediremi mediremi left a comment

Choose a reason for hiding this comment

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

LGTM ✔️

@cknitt
Copy link
Member

cknitt commented Jan 20, 2026

I would also make @rescript/runtime a peer dependency.

Comment on lines 44 to 66
let getString = (formData, name) => {
switch formData->getUnsafe(name) {
| Some(value) => Js.typeof(value) === "string" ? Some(value) : None
| Some(value) => typeof(value) === #string ? Some(value) : None
| _ => None
}
}

external _asFile: 'a => file = "%identity"

let getFile = (formData, name) => {
switch formData->getUnsafe(name) {
| Some(value) => Js.typeof(value) === "string" ? None : Some(value->_asFile)
| Some(value) => typeof(value) === #string ? None : Some(value->_asFile)
| _ => None
}
}

let getAll = (t, string) => {
t
->getAllUnsafe(string)
->Js.Array2.map(value => {
Js.typeof(value) === "string" ? String(value) : File(value->_asFile)
->Array.map(value => {
typeof(value) === #string ? String(value) : File(value->_asFile)
})
}
Copy link
Member

@tsnobip tsnobip Jan 20, 2026

Choose a reason for hiding this comment

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

why don't you use untagged variants here?

module FormData = {
  type t

  @unboxed
  type formValue =
    | String(string)
    | File(Js.File.t)

  @new external make: unit => t = "FormData"

  @send external append: (t, string, ~filename: string=?) => unit = "append"
  @send external delete: (t, string) => unit = "delete"
  @return(nullable) @send external get: (t, string) => option<formValue> = "get"
  @send external getAll: (t, string) => array<formValue> = "getAll"

  let getString = (formData, name) => {
    switch formData->get(name) {
    | Some(String(value)) =>  Some(value)
    | _ => None
    }
  }

  let getFile = (formData, name) => {
    switch formData->get(name) {
    | Some(File(value)) => Some(value)
    | _ => None
    }
  }

  @send external set: (string, string) => unit = "set"
  @send external has: string => bool = "has"
  // @send external keys: t => Iterator.t<string> = "keys";
  // @send external values: t => Iterator.t<value> = "values";
}

Copy link
Member Author

Choose a reason for hiding this comment

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

Because I do not care about the existing code. At lot of it can be improved, this is not what this PR is about.

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.

4 participants