-
Notifications
You must be signed in to change notification settings - Fork 3.3k
[cross_file] Adding custom sources to feed data to XFile
#6625
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
Conversation
- Added lazy-loading of a blob for the file content which fixes saveTo with html.dart when using a custom source - Fixed incorrect constructor in interface.dart - Moved TextXFileSource to common.dart file
(One of) The problems with XFile in its current version, is that it should have been modeled as an interface which people use to read the contents of the file, but then we would have per-platform versions of the creation of the file, so we can use the most appropriate version for each platform (from bytes, from a Blob, from a This PR may help by introducing the extra complexity of the This approach may also fail on the web, where the implementation of Anyway, what do you think @stuartmorgan? |
I have successfully kept only the source in the This would make the source manage the destination and we don't want that. |
From triage: I'll try to look at the design for this sometime in the next week. |
@stuartmorgan do ping me if you want to brainstorm over a call! |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry for the long delay on this. I think trying to shoehorn another, even more different source into the current design exacerbates the problems |
(triage) closing based on last comment |
I'm proposing a redesign of XFile that would make it very easy to add additional platform-specific types to XFile, so developers can pick the best type when creating an XFile (based on platform and data source), and consume those types transparently in a cross-platform way:
|
This PR adds a new constructor for
XFile
(XFile.fromCustomSource
) that only takes an implementation of a new abstract classXFileSource
. This new class enables users of thecross_file
plugin to construct anXFile
and have outputs totally managed by the user.I removed multiple TODOs as the version required to remove them was met.
New tests have also been added for this new constructor for both
dart:io
and web versions.NOTE: I got the web version tests to run in a suboptimal environment because of flutter/flutter-intellij#5550, although it seemed like it didn't change the behavior of the tests.
It seems like a move to a federated plugin for cross_file is in the works (flutter/flutter#91869), it could split
XFile
into multiple classes and break this PR. Should this feature be delayed for the move to a federated or do we merge now and see how it goes then improve it via feedback when the package will be federated?This PR will enable support for
content://
URIs to be implemented withXFile
s (It is the first step to close flutter/flutter#147037).Pre-launch Checklist
dart format
.)[shared_preferences]
pubspec.yaml
with an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.CHANGELOG.md
to add a description of the change, following repository CHANGELOG style.///
).If you need help, consider asking for advice on the #hackers-new channel on Discord.