playwright icon indicating copy to clipboard operation
playwright copied to clipboard

[Feature] Allow for customization of Babel configuration

Open D4N14L opened this issue 2 years ago • 6 comments
trafficstars

Reference: #7121, #11440

Background:

We have a large monorepo with various different projects included in it. Our monorepo does not use Babel, and instead uses TypeScript/Webpack directly due to the various performance improvements that can be had when taking those approaches. However, we do use Playwright with Babel, exclusively for the reason that the iteration cycle on Playwright tests is greatly reduced since there is no need to run a build before executing Playwright. We use a library package that uses test.extend(...) to provide custom fixtures for tests and various additional functionality. We bundle this library in order to reduce loading overhead and also to make the library easily portable (ex. for running Playwright tests during CI with assets built on a different machine). This has lead to seeing the following error:

[BABEL] Note: The code generator has deoptimised the styling of <path_to_bundled_file>.js as it exceeds the max of 500KB.

While this error is purely cosmetic, it would be good to avoid needing this error to be shown at all. To do that, we would either need:

  1. To default to true/false for the compact option provided to Babel, or
  2. To allow for ignoring specific files with Babel, since we know these files should already be transpiled (ex. via an ignores: ["node_modules"] Babel config option)

Unfortunately for both of these, there is no existing method to provide Babel configuration overrides.

Ask:

Allow for providing Babel configuration overrides using playwright.config.js.

While using compiled JS (as mentioned in #11440) is a workaround for this issue since it avoids Babel entirely, the quick iteration loop that Babel in Playwright allows for is a fantastic value-add and something we would love to continue using. On top of that, I'm actually not even able to use this approach, due to the issue pointed out in this comment: https://github.com/microsoft/playwright/issues/8798#issuecomment-1007472776. Lastly, in approaches like ours, we already know that the upstream dependencies should already be built. Why should we need to run our built dependencies through Babel and take that performance penalty when we know they should already be consumable? This also allows for much greater flexibility for approaches that do use Babel outside of Playwright.

D4N14L avatar May 07 '23 23:05 D4N14L

Context @pavelfeldman this is for the odsp-web team. We are happy to contribute a fix for this but we want to make sure there is some form of a design sign-off because what is proposed here requires a modification to the playwright configuration.

TheLarkInn avatar May 08 '23 15:05 TheLarkInn

Could you please follow the issue template and provide something we could run locally? I'm not sure I understand your configuration in full, not sure which parts of the e2e tests you compile yourselves vs in which parts you rely on Playwright babeling code. Are we processing it with Babel twice?

pavelfeldman avatar May 08 '23 16:05 pavelfeldman

Ok, I re-read this and I think I have a better idea on what you want. For now we can:

  • change the compact mode default
  • add a filter that allows ignoring processed code when transpiling

Does your processed code have source maps? We could use it as a hint when feciding whether to process it. We can also detect babel preamble - could you share the prefix of your processed file?

pavelfeldman avatar May 11 '23 14:05 pavelfeldman

I'll ping @D4N14L again he might not have seen the previous messages. But I think this probably takes us a step close to where we need to be.

TheLarkInn avatar May 11 '23 16:05 TheLarkInn

Apologies, was meaning to reply but got caught up with a few other things. @pavelfeldman that sounds pretty much exactly like what I'm looking for (currently I'm working around the warning with a PNPM patch that adds compact: false). I think the first point really fixes the main issue that I'm looking to work around with this feature request.

As for processed code, yes we do produce source maps to assist with debugging our local transpiled code when an error is thrown. Though, I think it would be great to be able to ignore Babel processing regardless of the presence of source maps. The processed code that is triggering the warning is all Webpack-bundled, so begins with the standard Webpack premable:

/******/ (() => { // webpackBootstrap
/******/ 	var __webpack_modules__ = ({
...

Having said that, it would be useful if we could instruct Babel to optionally ignore any JS that it encounters. allowing Babel to be used only as a middleman between TS test code and Playwright. Does that make sense?

D4N14L avatar May 11 '23 17:05 D4N14L

@pavelfeldman so I dug in a bit more on this, seems like the issue is that normally, node_modules should get excluded from the transform that Playwright performs. However, in monorepos like ours, we symlink in-workspace packages into the node_modules directory. Due to this, the check for if the file comes from node_modules here before transforming fails, since the resolved realpath of the file is actually in the repository (even though it would be expected to be compiled). Due to this, Babel is still run on these files.

D4N14L avatar May 15 '23 19:05 D4N14L