Skip to content

add a function to construct a url, and change how to two libs are built #317

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

add a function to construct a url, and change how to two libs are built #317

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
0 packages/url/js/URL.re → packages/url/js/URL_impl.re
View file
Open in desktop
File renamed without changes.
5 changes: 2 additions & 3 deletions packages/url/js/dune
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@
(modes melange)
(public_name server-reason-react.url_js)
(libraries melange.js)
(wrapped false)
Copy link
Collaborator

@pedrobslisboa pedrobslisboa Oct 23, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

We cannot remove the wrapped false as it enables the library to function as "universal".

If you don't have it the usage becomes Url_js.Url instead of only Url, which missmatches with the native library Url_native.Url

Copy link
Author

Choose a reason for hiding this comment

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

Its still unwrapped because it implements url which is a unwrapped library. If you try to add it back you get an error message.

(preprocess
(pps melange.ppx)))
(pps melange.ppx))
(implements url))

(copy_files# "../URL.rei")
Copy link
Collaborator

Choose a reason for hiding this comment

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

We also don't like copy_files, but until we have the dune single-context feature, we need it.
(Related https://github.com/ml-in-barcelona/server-reason-react/pull/317/files#r2453920368)

7 changes: 7 additions & 0 deletions packages/url/lib/URL.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
include URL_impl

let construct ?protocol ~hostname ?port ?pathname ?search ?hash ?password ?username () =
let apply_opt f o = match o with None -> Fun.id | Some x -> fun u -> f u x in
"https://example.com" |> makeExn |> apply_opt setProtocol protocol |> Fun.flip setHostname hostname
|> apply_opt setPort port |> apply_opt setPathname pathname |> apply_opt setSearch search |> apply_opt setHash hash
|> apply_opt setPassword password |> apply_opt setUsername username
Comment on lines +1 to +7
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm confused about the motivation of this feature. Can you explain better in the PR description?

0 packages/url/URL.rei → packages/url/lib/URL_impl.rei
View file
Open in desktop
File renamed without changes.
5 changes: 5 additions & 0 deletions packages/url/lib/dune
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
(library
Copy link
Collaborator

Choose a reason for hiding this comment

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

It shouldn't work with melange as it doesn't have the universal structure. Take a look here: https://ml-in-barcelona.github.io/server-reason-react/server-reason-react/how-to-organise-universal-code.html

(name url)
(public_name server-reason-react.url)
(wrapped false)
(virtual_modules URL_impl))
0 packages/url/native/URL.re → packages/url/native/URL_impl.re
View file
Open in desktop
File renamed without changes.
4 changes: 2 additions & 2 deletions packages/url/native/dune
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
(library
(name url_native)
(public_name server-reason-react.url_native)
(wrapped false)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as here:
https://github.com/ml-in-barcelona/server-reason-react/pull/317/files#r2453917671

(libraries uri js))
(libraries uri js)
(implements url))

(copy_files# "../URL.rei")
Loading