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

fix(neon): Fix SVG code compilation for web #1041

Merged
merged 1 commit into from
Nov 2, 2023
Merged

Conversation

provokateurin
Copy link
Member

Extracted from #372
This wasn't compiling before because it was relying on dart:io code internally.

Copy link
Member

@Leptopoda Leptopoda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the SvgLoader documentation:

/// A [BytesLoader] that parses a SVG data in an isolate and creates a
/// vector_graphics binary representation.

This means loading the file could happen in a separate isolate thus not blocking the main one.
I'd rather make a custom UniversalSvgFileLoader as each loader is only around 30 lines of code.

@provokateurin
Copy link
Member Author

The annoying thing is that this code isn't even executed on web since we have no push notification support there. Maybe we should instead use a stub on web to fix the compilation (conditional import if you know what I mean).

@Leptopoda
Copy link
Member

I don't know what makes most sense rn but this shouldn't even be a problem with #808.
Do you think it's worth stubbing?

@provokateurin
Copy link
Member Author

I think I will go with the stub for now and once we have the plugin system we can easily drop it.

@provokateurin
Copy link
Member Author

I went with your suggestion because creating the loader was very easy indeed. Just had to copy the SvgFileLoader and replace dart:io with universal_io.

@provokateurin provokateurin merged commit 64f1c18 into main Nov 2, 2023
9 checks passed
@provokateurin provokateurin deleted the fix/neon/svg-web branch November 2, 2023 07:44
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.

2 participants