Skip to content
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

[rescript] Not working with playwright #404

Open
searleser97 opened this issue Mar 26, 2024 · 6 comments
Open

[rescript] Not working with playwright #404

searleser97 opened this issue Mar 26, 2024 · 6 comments
Labels
bug Something isn't working rescript For `ReScript` target

Comments

@searleser97
Copy link

searleser97 commented Mar 26, 2024

Could you provide instructions on how to make it work with the definition files in the playwright repo ? -> https://github.com/microsoft/playwright/tree/main/packages/playwright-core/types

Do I need to run the command for just the main definition file ? or for every file ?

context: I have tried but I always get code that does not compile

I am using latest version of rescript btw (11.0.1)

@searleser97 searleser97 changed the title Not working with playwright [rescript] Not working with playwright Mar 26, 2024
@smorimoto smorimoto added the rescript For `ReScript` target label Mar 26, 2024
@searleser97
Copy link
Author

searleser97 commented Mar 27, 2024

So, after splitting the modules into different files, I see that it created modules with cyclic dependencies.

For example:

which creates something like the following in rescript:

module Request = {
  type t;
  @send external response: (t) => Promise.t<Null.t<Response.t>> = "response"
}

module Response = {
  type t;
  @send external request: (t) => Request.t = "request"
}

What would be the fix for those scenarios ?

@cannorin
Copy link
Member

In this case, ts2ocaml should emit those modules as recursive (module rec). Which preset and other options did you use for emitting the binding file? It would be a bug on ts2ocaml if you used --preset full.

@cannorin cannorin added the bug Something isn't working label Mar 30, 2024
@searleser97
Copy link
Author

I used that full option but didn't work, it does emit some recursive modules when it emits the resi file but it does not compile still

@searleser97
Copy link
Author

Were you able to replicate the issue ?

@cannorin
Copy link
Member

cannorin commented Apr 8, 2024

Sorry, I didn't have an enough free time recently to fully investigate it (a business/school year starts at April in my country). I'll look into it this week.

@cannorin
Copy link
Member

From what I can see, playwright-core contains mutually recursive source files types/structs.d.ts and types/types.d.ts, which is the main reason why the generated binding does not compile.

ReScript does not support mutually recursive source files, and, unfortunately, it is very difficult for ts2ocaml to fix it automatically. As a workaround, ts2ocaml provide an option --merge, which just merges all the input files at the risk of breaking relative imports and increasing the output size.

Luckily, playwright-core is small and simple enough to apply the --merge option. Try running this:

$ ts2ocaml --preset minimal --merge node_modules/playwright-core/index.d.ts node_modules/playwright-core/types/*.d.ts

Then you would need to modify the output, as it contains several broken import statements and type names:

  1. Assuming you have rescript-nodejs installed, replace the import statements of Node API with the following:
    /* import { ChildProcess } from 'child_process'; */
    module ChildProcess = NodeJs.ChildProcess
    /* import { EventEmitter } from 'events'; */
    module EventEmitter = NodeJs.EventEmitter.Make()
    /* import { Readable } from 'stream'; */
    module Readable = NodeJs.Stream.Readable
    /* import { ReadStream } from 'fs'; */
    module ReadStream = NodeJs.Fs.ReadStream
  2. Replace URL.t with NodeJs.Url.t.
  3. Replace Readable.t with Readable.t<'t> to match rescript-nodejs's implementation.
  4. Replace SVGElement.t with something else or just remove it (ReScript's DOM library doesn't provide a binding for some reason).

I've successfully compiled it in PR #412 with the above options and modifications:
https://github.com/ocsigen/ts2ocaml/actions/runs/8773991443/job/24074711135?pr=412#step:6:1102

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working rescript For `ReScript` target
Projects
None yet
Development

No branches or pull requests

3 participants