accessibility-insights-web icon indicating copy to clipboard operation
accessibility-insights-web copied to clipboard

tech debt: report generation should embed styles using a more robust mechanism

Open dbjorge opened this issue 4 years ago • 1 comments
trafficstars

Describe the ~bug~ technical debt

Today, we use a custom Grunt embed-styles task to implement generating report HTML which embeds the CSS bundles produced by web's webpack rules. This isn't a very robust solution; it requires us to handle quote/line escaping manually in a way that is reliant on implementation details of both webpack and our CSS loader chain. This has caused issues which are very challenging/confusing to debug (eg, #4719). It also regularly trips security warnings, since manual escaping logic is error-prone (again, see #4719) and since it forces us to use React's dangerouslySetInnerHTML.

It would be nice to rip out the custom embed-styles logic and replace it with a more robust option for embedding the CSS in the HTML templates our reports use. Some options for this include:

  • Use css-loader's exportType: 'string' to replace the embed-styles marker strings more-or-less as-is. This is probably lowest-cost, but doesn't eliminate the dangerouslySetInnerHTML usage.
  • Use html-webpack-plugin and html-webpack-inline-style-plugin to generate prebuilt HTML templates that include embedded styles, and use that as input to our existing report renderStatic call.
  • Use html-webpack-plugin and html-webpack-inline-style-plugin, but at build time generate the whole HTML file modulo a templated spot to insert input data into, replacing renderStatic. This would eliminate some of the technical debt around maintaining multiple implementations of the dynamic report expand/collapse components, but it probably isn't technically feasible unless we accept making the report files much larger than they currently are.

dbjorge avatar Sep 23 '21 23:09 dbjorge

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!

ghost avatar Sep 23 '21 23:09 ghost

After reviewing the full list of issues in our backlog, we have concluded that we will not be addressing this issue.

DaveTryon avatar Jun 29 '23 23:06 DaveTryon