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

Removing unsued input styles in function exports #176

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

fdoflorenzano
Copy link
Contributor

@fdoflorenzano fdoflorenzano commented May 11, 2023

This is a first direction of fixing an old bug introduced with custom inputs.

This was first noted when working with custom inputs that used some of Mechanic's ui-components. Some SVG based design functions included in the project that included the custom inputs, would fail at export (either PNG or SVG) with the following message:

Uncaught (in promise) Error: Pseudo-elements are not supported by css-select.

Through some digging, it was found that this is thrown by SVGO (that uses css-select) when trying to compute optimizations over the resulting SVG. It threw that because it turns out CSS meant for input elements (like text field, number field, etc...) got bundled into the SVG being exported, and those included pseudo-elements.

This happens because a section was added when adding CSS support for SVG, that clones all style in the design functions iframe into the SVG, so that exported SVG actually included styles defined in the design function. But, it wasn't considered at the time that this would carry over any other styles that may not even be for the design function, like the ones from custom inputs. Custom inputs style is there in the first place because Mechanic adds dependencies to the script run in the iframe that imports the inputs code, so it can run validations and other value things, but therefore also adds any style associated with inputs.

This problem was first posted as #129. I go back and forth on whether if it's an actual problem that the input's CSS gets to the iframe. What is a problem for sure is that it makes it to the export.

This draft includes a potential fix focusing on the export step. Search through style rules that are included in the iframe, and skipping any that has rules that don't match to anything in the export or raise error when searching. Those error might be because of badly written CSS or unsupported selector (like pseudo-elements).

Adds two dependencies that are already included by SVGO: css-select (to query rule selectors over the SVG) and css-tree (to parse and write CSS strings).

Let me know what you think! I will open a new fresh PR with cleaner commits if we decide to take it in.

@netlify
Copy link

netlify bot commented May 11, 2023

Deploy Preview for dsi-logo-maker ready!

Name Link
🔨 Latest commit 93f3a60
🔍 Latest deploy log https://app.netlify.com/sites/dsi-logo-maker/deploys/6463d57aa57e0d00086b1441
😎 Deploy Preview https://deploy-preview-176--dsi-logo-maker.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@fdoflorenzano fdoflorenzano changed the title WIP Removing unsued input styles in function exports Removing unsued input styles in function exports May 16, 2023
@runemadsen
Copy link
Contributor

Thanks for this! I think this solution is fine as a workaround of the problem, so I have no problem with this going into the codebase. That being said, I would prefer that we prevent the CSS from being in iframe in the first place, since it doesn't really seem like it's needed there. Can you point me towards the exact place where the CSS is being injected?

@fdoflorenzano
Copy link
Contributor Author

That being said, I would prefer that we prevent the CSS from being in iframe in the first place, since it doesn't really seem like it's needed there.

Yeah, that's the part where I kept going back on forth on. Cause in a way what's behind that is just webpack adds it following the dependency chain. If the custom input imports whatever CSS for no reason related to the input validations, that would also be added, and it would make sense (in the sense that, it's a dependency, so it should be there).

Can you point me towards the exact place where the CSS is being injected?

Sure, it's a bit of a maze at this point. The code that gets run directly in the iframe is this section.

  • Which imports in the first line the results generated here. That goes through the input definitions that checks that stuff makes sense (just in the input defs). That script is here.
  • Then, the iframe script, runs the setUp function defined here. Which in some other stuff, would run the input validation functions for the values passed from the UI.

We could try to figure out how to refactor this so that the iframe is able to call the validations for the passed values, without directly importing that codebase.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants