Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor: latex and qrcode pipes #2530

Draft
wants to merge 30 commits into
base: master
Choose a base branch
from
Draft

Conversation

chrismclarke
Copy link
Member

@chrismclarke chrismclarke commented Nov 15, 2024

PR Checklist

  • PR title descriptive (can be used in release notes)

TODO

Description

Refactor latex and qrcode pipes to instead be called from their corresponding components.

Review Notes

Confirm qrcode and latex example component sheets working as expected (looks correct to me)

Dev Notes

As there are currently no other components that attempt to parse qrcode or latex values it makes sense to keep the code component-specific, and hence will be able to tree-shake in/out during an optimised build depending on component use.

I've moved the latex code to a util file so that if other components did want to implement latex they could still call the method (which would by default also be included in optimised build whenever the component is, without the need to define any implicit dependencies due to the direct import).

I haven't refactors the markdown pipe as it is currently in use by both button and text component which are included in every build regardless of optimisation (so will break the pipe for any build that tries to remove). It's also used by tile and simple checkbox components.

In the future likely the best way to remove would probably be some sort of parameter_list setting to specify format: markdown, which can be parsed in the template service before providing the value at the component level. We could then use report analysis to see if authors ever use, and remove formatter as necessary. Would likely also want to include a global option to enable by default on all components (if desirable). But all this is quite overkill for now, so just sticking to the easier-to-manage pipes.

Git Issues

Closes #

Screenshots/Videos

Build Analysis - qrcode and katex dependencies no longer appearing when compared to #2520 snapshot of plh_kids_kw build. Core bundle reduced additional 300kb/~10%
image

@github-actions github-actions bot added scripts Work on backend scripts and devops maintenance Core updates, refactoring and code quality improvements labels Nov 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Core updates, refactoring and code quality improvements scripts Work on backend scripts and devops
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant