Skip to content

Conversation

@43081j
Copy link

@43081j 43081j commented Nov 5, 2025

This exports launch - a function which takes LaunchOptions and runs
the browser the same way the CLI does.

The CLI has then moved to lib/cli.js and calls launch internally
with normalized options.

This means the various CLI arg normalizations (e.g. parsing cookie
objects) happen only in the CLI, while the underlying code expects a
strong type.

This exports `launch` - a function which takes `LaunchOptions` and runs
the browser the same way the CLI does.

The CLI has then moved to `lib/cli.js` and calls `launch` internally
with normalized options.

This means the various CLI arg normalizations (e.g. parsing cookie
objects) happen only in the CLI, while the underlying code expects a
strong type.
@43081j 43081j marked this pull request as draft November 5, 2025 11:43
@43081j
Copy link
Author

43081j commented Nov 5, 2025

i see #26 already does this, completely missed that!

close this one if you wish. binned code is still good code 😀

i had this locally a few days as i wanted to run it myself without the CLI, so it was still useful 👍

edit:

here's a few notable differences that may be worth pulling into the other one:

  • LaunchOptions has a strong type and leans on the playwright types
  • launch shouldn't do any normalization, only the CLI should normalize (since you'd expect strong types to be passed to the programmatic API)
  • the various JSON.parse calls have been dropped here since we pass the already parsed and normalized values in
  • flags is a CLI only option and shouldn't reach launch (its just a way to define args)
  • bin has been set in the package.json
  • the various parseFloat have been removed as they are also just part of CLI normalization

@dangayle FYI in case you want to do any of this in your branch

@sergeychernyshev
Copy link
Member

@43081j I merged @dangayle's PR and added a couple of your fixes.

Can you take another look and see what else is missing?

Re type definitions, we are hoping to move the code to TypeScript in the near future as well so it will be a bit more strict and provide good typing and code completion for programmatic execution.

@43081j
Copy link
Author

43081j commented Nov 9, 2025

I still don't think it's right that the launch function normalises options. If this was in typescript already, it would be a lot clearer why.

The programmatic usage should accept a strong type, but the cli can't do that since all parameters are strings or booleans. I don't think we should be letting the cli normalisation leak into the underlying API

By doing so, it means everything is stringly typed right now (you can pass all of the complex objects and numbers as strings programmatically)

@sergeychernyshev
Copy link
Member

I agree that preemptive normalization is a source of long-lasting bugs and maintenance nightmare after you get a lot of implementations that “just worked”.

Could you apply the type declarations and move normalization to CLI only in this PR to the new code that we got?

I’ll be happy to review and merge it.

@dangayle
Copy link
Contributor

dangayle commented Nov 9, 2025 via email

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.

3 participants