playwright icon indicating copy to clipboard operation
playwright copied to clipboard

feat: support importing client-only files (like scss) for ESM

Open AlessioGr opened this issue 1 year ago • 4 comments

Related issue: #26822 (doesn't close it, being able to provide your own loader would still be cool and needed)

Currently, if you import client extension files like scss within playwright and use ESM, it throws an "Unknown file extension: .scss" error. This PR fixes this issue.

This is unfortunate, because our own webpack loader is indeed able to import and bundle those scss files. Back when we used CJS, we were able to skip requiring client files using this simple trick.

With ESM imports, this is no longer possible and there is no way for us to get playwright to support it. The "ESM-way" to do this is to use a custom loader which skips resolving those client files and loads them as raw JSON. That way, they won't error out and a bundler can then handle them accordingly. This is what this PR does.

Currently, we are patching playwright in order to get playwright to actually work in our project: https://github.com/payloadcms/payload/blob/alpha/patches/playwright%401.42.1.patch. Getting client file imports to work is one out of three things we had to patch in order for playwright to support ESM. This PR will relieve some burden for us to patch it after each version, until hopefully in the future this is not needed at all anymore.

I'm aware the alternative to this is to either compile our project beforehand and run playwright on dist, or to run next.js in a separate process and make sure no client files are imported within playwright itself. This, however, is a very bad developer experience for us, especially since we cannot use a debugger that way.

AlessioGr avatar Mar 20 '24 00:03 AlessioGr

@microsoft-github-policy-service agree company="Payload CMS, Inc."

AlessioGr avatar Mar 20 '24 00:03 AlessioGr

Test results for "tests 1"

4 interrupted :warning: [playwright-test] › config.spec.ts:395:5 › should work without config file
:warning: [playwright-test] › deps.spec.ts:141:5 › should not run project if dependency failed (2)
:warning: [playwright-test] › reporter-blob.spec.ts:153:1 › should merge into html with dependencies
:warning: [playwright-test] › reporter-blob.spec.ts:239:1 › should merge blob into blob

70 passed, 1286 did not run :heavy_check_mark::heavy_check_mark::heavy_check_mark:

Merge workflow run.

github-actions[bot] avatar Mar 20 '24 00:03 github-actions[bot]

Test results for "tests 1"

5 interrupted :warning: [installation tests] › driver-should-work.spec.ts:18:5 › driver should work
:warning: [playwright-test] › config.spec.ts:290:5 › should filter by project wildcard and exact name
:warning: [playwright-test] › deps.spec.ts:19:5 › should run projects with dependencies
:warning: [playwright-test] › reporter-blob.spec.ts:153:1 › should merge into html with dependencies
:warning: [playwright-test] › reporter-blob.spec.ts:239:1 › should merge blob into blob

61 passed, 1 skipped, 1341 did not run :heavy_check_mark::heavy_check_mark::heavy_check_mark:

Merge workflow run.

github-actions[bot] avatar Mar 20 '24 00:03 github-actions[bot]

Test results for "tests 1"

5 flaky :warning: [chromium-library] › library/tracing.spec.ts:243:5 › should not include trace resources from the previous chunks
:warning: [chromium-library] › library/tracing.spec.ts:243:5 › should not include trace resources from the previous chunks
:warning: [firefox-library] › library/tracing.spec.ts:243:5 › should not include trace resources from the previous chunks
:warning: [chromium-library] › library/tracing.spec.ts:243:5 › should not include trace resources from the previous chunks
:warning: [playwright-test] › ui-mode-test-screencast.spec.ts:21:5 › should show screenshots

26839 passed, 620 skipped :heavy_check_mark::heavy_check_mark::heavy_check_mark:

Merge workflow run.

github-actions[bot] avatar Mar 20 '24 01:03 github-actions[bot]

I'm aware the alternative to this is to either compile our project beforehand and run playwright on dist, or to run next.js in a separate process and make sure no client files are imported within playwright itself. This, however, is a very bad developer experience for us, especially since we cannot use a debugger that way.

This would still be our recommendation. So far your issue did not collect the number of votes for us to consider the PR.

pavelfeldman avatar Mar 27 '24 17:03 pavelfeldman