Skip to content

Conversation

ThijsBroersen
Copy link

I could not find any existing JSEnv which allows me to properly test against some browser features and am using scala-js-env-playwright for a while now. The current version is still a snapshot so this PR is not ready to be merged, but I would love some feedback on what is required to get this feature as part of Mill. Currently I am using a custom build mill-assembly for my projects.

Here a draft of Playwright support for Scala-js testing.

I was also wondering why isn't Mill supporting just a raw JSEnv? Any custom JSEnv implemention now first requires Mill to include it..

@lefou
Copy link
Member

lefou commented Aug 29, 2025

I was also wondering why isn't Mill supporting just a raw JSEnv? Any custom JSEnv implemention now first requires Mill to include it

I agree to it. It's nice to have out-of-the-box support for popular JSEnvs, but this should not be a closed set. Anyway, this should be reported/fixed in a seperate issue/PR.

@ThijsBroersen ThijsBroersen force-pushed the feat-support-for-playwright-testing-for-scalajs branch from 814d451 to 032158d Compare September 13, 2025 12:26
@ThijsBroersen ThijsBroersen marked this pull request as ready for review September 13, 2025 12:28
Comment on lines +220 to +226
val defaultWebkitLaunchOptions = List(
"--disable-extensions",
"--disable-web-security",
"--allow-running-insecure-content",
"--disable-site-isolation-trials",
"--allow-file-access-from-files"
)
Copy link
Member

Choose a reason for hiding this comment

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

Where do these options come from? Can you link some documentation to ease future maintenence?

Copy link
Author

Choose a reason for hiding this comment

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

These are the defaults we currently have in the plugin. Shall I just add some documentation referring to the plugin repo?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, that helps maintaining them.

Copy link
Author

Choose a reason for hiding this comment

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

I cannot find any docs related to using the jsEnvConfig task. Where shall we add something?

Copy link
Author

Choose a reason for hiding this comment

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

I added refs to the plugin repo source in code docs.

@lolgab
Copy link
Member

lolgab commented Sep 16, 2025

I was also wondering why isn't Mill supporting just a raw JSEnv? Any custom JSEnv implemention now first requires Mill to include it

I agree to it. It's nice to have out-of-the-box support for popular JSEnvs, but this should not be a closed set. Anyway, this should be reported/fixed in a separate issue/PR.

The rationale to not depend on the scalajs artifact was binary compatibility. If we used scalajs-jsenvs in mill it means that if scala-jsenvs gets a version 2.0 it's a problem in Mill since it would not be binary compatible with the one we shipped. in

@ThijsBroersen
Copy link
Author

ThijsBroersen commented Sep 16, 2025

I was also wondering why isn't Mill supporting just a raw JSEnv? Any custom JSEnv implemention now first requires Mill to include it

I agree to it. It's nice to have out-of-the-box support for popular JSEnvs, but this should not be a closed set. Anyway, this should be reported/fixed in a separate issue/PR.

The rationale to not depend on the scalajs artifact was binary compatibility. If we used scalajs-jsenvs in mill it means that if scala-jsenvs gets a version 2.0 it's a problem in Mill since it would not be binary compatible with the one we shipped. in

I already assumed that. However, adding new test-environments is now only possible by modifying the build tool. Would there be a path possible where this could be pulled in via a plugin perhaps? (And perhaps would should open a discussion to answer this)

@ThijsBroersen
Copy link
Author

Could there be made a decision on what is going to happen with this PR? Rolling my own Mill because I need Playwright is quite annoying.
I started a discussion wrt to any JSEnv support: #5936

@lihaoyi
Copy link
Member

lihaoyi commented Sep 29, 2025

At a first glance this looks reasonable. @lefou @lolgab any more concerns? If not we can merge it

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