[RFC] [cross_file] New architecture. #7591
Conversation
|
/cc @stuartmorgan this is my idea for the "next" version of the x-files. PTAL and let me know what you think, or if you want this to go in another direction :)
|
|
|
||
| @override | ||
| Future<void> saveTo(String path) async { | ||
| final File fileToSave = File(path); |
There was a problem hiding this comment.
This is lifted from the old implementation but... should the File be created from path + this.name?
packages/cross_file/lib/src/implementations/web_bytes_x_file.dart
Outdated
Show resolved
Hide resolved
|
|
||
| /// A CrossFile backed by a dart:io [File]. | ||
| /// | ||
| /// (Mobile-only). |
There was a problem hiding this comment.
Don't forget desktop :)
| /// (Mobile-only). | |
| /// This is only supported on non-web platforms. |
| /// The [path] variable is ignored. | ||
| @override | ||
| Future<void> saveTo(String path) async { | ||
| // Save a Blob to file... |
There was a problem hiding this comment.
This comment seems a bit redundant.
| objects and their URLs. | ||
|
|
||
| It seems that Safari hangs when reading Blobs larger than 4GB (your app will stop | ||
| without returning any data, or throwing an exception). | ||
|
|
||
| This package will attempt to throw an `Exception` before a large file is accessed |
There was a problem hiding this comment.
Just to be sure, was this comment removed, because we now refer to the browser's behavior? Or was this fixed in recent versions of Safari?
|
|
||
| Example: | ||
| In order to instantiate a new `XFile`, import the correct factory class, | ||
| either from `package:cross_file/native/factory.dart` (for native development) or |
There was a problem hiding this comment.
Small drive-by comment:
Since the two implementations now have platform-specific imports (dart:io or package:web)
users who take the imports at face value might run into compilation issues, because they lack a conditional import?
Can we provide guidance on how to avoid this? With the legacy API we had the File or Blob types as internals and not on the public interface, which allowed for the plugin to provide a conditional import shim.
|
Quick note to say I haven't forgotten about this, I just haven't had time to fully context switch to give this the attention it will need. I'll try to do it within the next week. |
This PR attempts to solve the current issues of
XFileby slightly re-architecting it.In this PR:
XFileis now an interface that does not have any code, except for the two "legacy" constructors that are maintained (but deprecated) for backwards-compatibility reasons.XFileinstances are now created from the platform-specific (nativevsweb) factories.This does not remove
saveAsfrom theXFileinterface.Note
This is a breaking change because I've made the oldXFileconstructors deprecated (and throwy) but if we want to, we could possibly re-implement the "old" constructors with the new implementations and a few conditional imports?This is not a breaking change anymore. Old tests continue passing (
test/legacy*), and new tests contain minor changes.Anyway, just a RFC!
Pre-launch Checklist
dart format.)[shared_preferences]pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.CHANGELOG.mdto add a description of the change, following repository CHANGELOG style, or this PR is exempt from CHANGELOG changes.///).If you need help, consider asking for advice on the #hackers-new channel on Discord.