Engineering: explore migration to esbuild

[waabid]

trafficstars

I'm creating this issue to track an exploration into migrating this project from webpack to esbuild.

Latest branch: https://github.com/waabid/accessibility-insights-web/tree/esbuildTry2

Current status: local dev build is working in this branch (yarn build)

Some stats based on yarn build on my machine:

• esbuild: 15s on average.
• webpack: 45s on average.

Changes that were made for esbuild to work

• All style imports must now be represented as: import stylesFromX from 'path/to/X.scss';
  • Previously, our imports were mostly like import * as stylesFromX from 'path/to/X.scss'; esbuild had issues parsing this format. I believe this has to do with how the modules are created (only have default exports) but not 100% sure if this change is required. If you would like to look into this, make sure to validate the produced JS bundles/local dev build: it will build cleanly but the class names may not transfer from the scss parsing. This can also be seen when the logLevel is set to debug.
  • Similarly, changed how UAParser is imported.
• Created style-plugin.js which is now the home to our css-modules solution.
• Embedding styles into our source map files fixed issues that generally plagued our sourcemap usage but more so for esbuild.
• Using dynamic imports in injected code prevented issues that seemed to be prevented by multiple injections (coincidentally happens in cross-frame e2e test).
• Esbuild doesn't support multiple entry-points creating one bundle file. To get around this, we:
  • moved code from stylesheet-init.ts to client-init.ts
  • created new files pertaining to react-devtools where we would import the devtools and our init files and in our esbuild configuration we now specify these new init files when react-devtools is present.

TODO

• [x] Appropriately name commands: currently it's all still named after webpack.
• [x] Figure out when/where we want to run type-checking: esbuild does not type check when compiling so we'll want to do that somewhere.
• [x] esbuild with tsc running sync: 35s on average. While this is only a modest decrease, I imagine we likely only run this for our CI/PR builds and likely do not include it in the local dev build script anyways.
• Ensuring the other packages being built by webpack can be built as well.
  • [x] web extension
  • [x] report package
  • [ ] ui package
  • [ ] unified
    • Running into errors at runtime. See here: https://github.com/microsoft/accessibility-insights-web/issues/5412#issuecomment-1128338484

Nice to have

• Implement watch for our style-plugin
• Implement caching for our style-plugin

[ghost]

This issue has been marked as ready for team triage; we will triage it in our weekly review and update the issue. Thank you for contributing to Accessibility Insights!

[waabid]

Update: with a minor change, was able to validate the branch on Windows as well.

Started looking into using the "minified" version of the bundles and started seeing an error in details view (but everything else seems functional). It's an error saying that axe is not a function ... which likely means there's an import issue somewhere.

[waabid]

On further investigation, this seems to be related to this issue: https://github.com/dequelabs/axe-core-npm/issues/479

Updating to axe-core 4.4.0 fixed the build with minified JS.

[waabid]

Did some further work on this.

• Unified doesn't seem to work at this moment: unclear what the issue is or what it's with. Ultimately, we can execute this change piecemeal, meaning unified can continue to use webpack until we figure out the issues.
  • Supporting unified did mean that I needed to update some further style imports.
  • Supporting our webpack config also meant removing the 'namedExports' flag for css-loader, since we no longer need it as due to the new way of importing our styles.
  • The error I'm currently seeing is :
    • Electron failed to install correctly, please delete node_modules/electron and try installing again. It's unclear what is causing this. Debugging further resulted in electron expecting some file to exist in our bundle folder but it never does. Looking at what webpack produced ... this text for the error never even appears in the old bundle. Kind of odd. Must mean the webpack bundling does not bundle electron?
• Can now specify prod but running into axe is not a function error again ... but with the newer version of axe-core. Needs further investigation.

[waabid]

Investigation on the minified axe-core producing axe is not a function error.

• I still do not fully understand what's causing this issue. It could be a bad interaction with how esbuild minifies code. It specifically mentions in the documentation that minification is not always safe (https://esbuild.github.io/api/#minify-considerations) so that could be a reason. We can look into raising an issue with esbuild but I think it'd be best to do that with a simpler repro in CodePen.
• Potential solution to look into: finding a way to not minify axe code at all! At the moment, I do not believe escode supports this but worth looking into further. An option for this might be to write a plug-in that does it.
• Workaround: don't minify the detailsview entry point which is the only place where this error occurs.
  • Downside for this would be the increased bundle sizes.
    • webpack/bundle minified folder size: 13.1 MB
    • esbuild/bundle minified folder size: 12.6 MB
    • esbuild/bundle minified folder size (with detailsview.bundle.js NOT minified): 15.7 MB

I've currently pushed the workaround to the branch but will likely need discussion on what we want to do and if we proceed, likely refactor that code to be a bit more readable.

[waabid]

Update:

• Manifest V3 dev build is now functional
• Report package is now functional

I think the next step will likely be to "productionize" the changes necessary for the web extension alone and then incrementally work on migrating other parts of the project (report, unified, ui) to esbuild in the main branch.

[waabid]

I've updated the parent to reflect current status of the esbuild migration.

Re: axe is not a function

• This error could've easily been avoided had I used the IIFE format in esbuild, since that is best for code running in browsers, which we are. ESM should be used for libraries that we are shipping (report, ui, being likely candidates).

[DaveTryon]

@madalynrose, will you please estimate the cost of this? We'll probably convert it to a feature if it's too big. Thanks!

[DaveTryon]

This should be a feature, migrated to ADO. @peterdur to make the feature, then we'll close this once the feature exists

[peterdur]

Work now tracked as ADO feature 2082104.