-
Notifications
You must be signed in to change notification settings - Fork 3.3k
[RFC] [cross_file] New architecture. #7591
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
base: main
Are you sure you want to change the base?
Changes from all commits
c1a7d31
f4f504e
285d994
6bfc7ac
4ae170f
0f8d772
b0a34e8
8ebf101
89f9b8f
c574bdb
a0e254b
45d9f06
1c3b4cf
14001c9
e0fa2e6
0873768
dbaabeb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,15 +4,38 @@ An abstraction to allow working with files across multiple platforms. | |
|
||
## Usage | ||
|
||
Import `package:cross_file/cross_file.dart`, instantiate a `XFile` | ||
using a path or byte array and use its methods and properties to | ||
access the file and its metadata. | ||
Many packages use `XFile` as their return type. In order for your | ||
application to consume those files, import | ||
`package:cross_file/cross_file.dart`, and use its methods and properties | ||
to access the file data and metadata. | ||
|
||
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems like we could just do this automatically from There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The problem with conditional exports/imports is that the analyzer freaks out when the APIs are different, which in this case they are (see this comment). Or do you have something else in mind? |
||
`package:cross_file/web/factory.dart` (for web development), and use the factory | ||
constructor more appropriate for the data that you need to handle. | ||
|
||
The library currently supports factories for the | ||
following source types: | ||
|
||
|| **native** | **web** | | ||
|-|------------|---------| | ||
| `UInt8List`| `fromBytes` | `fromBytes` | | ||
| `dart:io` [`File`][dart_file] | `fromFile` | ❌ | | ||
| Filesystem path | `fromPath` | ❌ | | ||
| Web [`File`][mdn_file] | ❌ | `fromFile` | | ||
| Web [`Blob`][mdn_blob] | ❌ | `fromBlob` | | ||
| `objectURL` | ❌ | `fromObjectUrl` | | ||
|
||
[dart_file]: https://api.dart.dev/stable/3.5.2/dart-io/File-class.html | ||
[mdn_file]: https://developer.mozilla.org/en-US/docs/Web/API/File | ||
[mdn_blob]: https://developer.mozilla.org/en-US/docs/Web/API/Blob | ||
|
||
|
||
### Example | ||
|
||
<?code-excerpt "example/lib/readme_excerpts.dart (Instantiate)"?> | ||
```dart | ||
final XFile file = XFile('assets/hello.txt'); | ||
final XFile file = XFileFactory.fromPath('assets/hello.txt'); | ||
|
||
print('File information:'); | ||
print('- Path: ${file.path}'); | ||
|
@@ -27,16 +50,12 @@ You will find links to the API docs on the [pub page](https://pub.dev/packages/c | |
|
||
## Web Limitations | ||
|
||
`XFile` on the web platform is backed by [Blob](https://api.dart.dev/be/180361/dart-html/Blob-class.html) | ||
`XFile` on the web platform is backed by `Blob` | ||
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We don't explicitly throw the "4 Gigabyte" exception; the browser may fail in unexpected ways (depending on what we end up doing with the bytes of the file) |
||
from Safari (if its size is known beforehand), so that case can be handled | ||
programmatically. | ||
|
||
### Browser compatibility | ||
|
||
[](https://caniuse.com/blobbuilder) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,57 @@ | ||
// Copyright 2013 The Flutter Authors. All rights reserved. | ||
// Use of this source code is governed by a BSD-style license that can be | ||
// found in the LICENSE file. | ||
|
||
import 'dart:io'; | ||
import 'dart:typed_data'; | ||
|
||
import '../cross_file.dart'; | ||
import '../src/implementations/io_bytes_x_file.dart'; | ||
import '../src/implementations/io_x_file.dart'; | ||
|
||
/// Creates [XFile] objects from different native sources. | ||
abstract interface class XFileFactory { | ||
/// Creates an [XFile] from a `dart:io` [File]. | ||
/// | ||
/// Allows passing the [mimeType] attribute of the file, if needed. | ||
static XFile fromFile( | ||
File file, { | ||
String? mimeType, | ||
}) { | ||
return IOXFile( | ||
file, | ||
mimeType: mimeType, | ||
); | ||
} | ||
|
||
/// Creates an [XFile] from a `dart:io` [File]'s [path]. | ||
/// | ||
/// Allows passing the [mimeType] attribute of the file, if needed. | ||
static XFile fromPath( | ||
String path, { | ||
String? mimeType, | ||
}) { | ||
return IOXFile.fromPath( | ||
path, | ||
mimeType: mimeType, | ||
); | ||
} | ||
|
||
/// Creates an [XFile] from an array of [bytes]. | ||
/// | ||
/// Allows passing the [mimeType], [displayName] and [lastModified] attributes | ||
/// of the file, if needed. | ||
static XFile fromBytes( | ||
Uint8List bytes, { | ||
String? mimeType, | ||
String? displayName, | ||
DateTime? lastModified, | ||
}) { | ||
return BytesXFile( | ||
bytes, | ||
mimeType: mimeType, | ||
displayName: displayName, | ||
lastModified: lastModified, | ||
); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,64 @@ | ||
// Copyright 2013 The Flutter Authors. All rights reserved. | ||
// Use of this source code is governed by a BSD-style license that can be | ||
// found in the LICENSE file. | ||
|
||
import 'dart:convert'; | ||
import 'dart:typed_data'; | ||
|
||
import '../x_file.dart'; | ||
|
||
/// The shared behavior for all byte-backed [XFile] implementations. | ||
/// | ||
/// This is an almost complete XFile implementation, except for the `saveTo` | ||
/// method, which is platform-dependent. | ||
abstract class BaseBytesXFile implements XFile { | ||
/// Construct an [XFile] from its data [bytes]. | ||
BaseBytesXFile( | ||
this.bytes, { | ||
String? mimeType, | ||
String? displayName, | ||
DateTime? lastModified, | ||
}) : _mimeType = mimeType, | ||
_displayName = displayName, | ||
_lastModified = lastModified; | ||
|
||
/// The binary contents of this [XFile]. | ||
final Uint8List bytes; | ||
final String? _mimeType; | ||
final String? _displayName; | ||
final DateTime? _lastModified; | ||
|
||
@override | ||
Future<DateTime> lastModified() async { | ||
return _lastModified ?? DateTime.now(); | ||
} | ||
|
||
@override | ||
String? get mimeType => _mimeType; | ||
|
||
@override | ||
String get path => ''; | ||
|
||
@override | ||
String get name => _displayName ?? ''; | ||
|
||
@override | ||
Future<int> length() async { | ||
return bytes.length; | ||
} | ||
|
||
@override | ||
Future<String> readAsString({Encoding encoding = utf8}) async { | ||
return encoding.decode(bytes); | ||
} | ||
|
||
@override | ||
Future<Uint8List> readAsBytes() async { | ||
return bytes; | ||
} | ||
|
||
@override | ||
Stream<Uint8List> openRead([int? start, int? end]) async* { | ||
yield bytes.sublist(start ?? 0, end ?? bytes.length); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,150 @@ | ||
// Copyright 2013 The Flutter Authors. All rights reserved. | ||
// Use of this source code is governed by a BSD-style license that can be | ||
// found in the LICENSE file. | ||
|
||
import 'dart:async'; | ||
import 'dart:convert'; | ||
import 'dart:typed_data'; | ||
|
||
import 'package:web/web.dart'; | ||
|
||
import '../web_helpers/web_helpers.dart'; | ||
import '../x_file.dart'; | ||
|
||
/// The metadata and shared behavior of a [blob]-backed [XFile]. | ||
abstract class BaseBlobXFile implements XFile { | ||
/// Store the metadata of the [blob]-backed [XFile]. | ||
BaseBlobXFile({ | ||
String? mimeType, | ||
String? displayName, | ||
DateTime? lastModified, | ||
}) : _mimeType = mimeType, | ||
_lastModified = lastModified ?? DateTime.fromMillisecondsSinceEpoch(0), | ||
_name = displayName; | ||
|
||
final String? _mimeType; | ||
final String? _name; | ||
final DateTime _lastModified; | ||
|
||
/// Asynchronously retrieve the [Blob] backing this [XFile]. | ||
/// | ||
/// Subclasses must implement this getter. All the blob accesses on this file | ||
/// must be implemented off of it. | ||
Future<Blob> get blob; | ||
|
||
@override | ||
String? get mimeType => _mimeType; | ||
|
||
@override | ||
String get name => _name ?? ''; | ||
|
||
@override | ||
String get path => ''; | ||
|
||
@override | ||
Future<DateTime> lastModified() async => _lastModified; | ||
|
||
@override | ||
Future<Uint8List> readAsBytes() async { | ||
return blobToBytes(await blob); | ||
} | ||
|
||
@override | ||
Future<int> length() async => (await blob).size; | ||
|
||
@override | ||
Future<String> readAsString({Encoding encoding = utf8}) async { | ||
return encoding.decode(await readAsBytes()); | ||
} | ||
|
||
// TODO(dit): https://github.com/flutter/flutter/issues/91867 Implement openRead properly. | ||
@override | ||
Stream<Uint8List> openRead([int? start, int? end]) async* { | ||
final Blob browserBlob = await blob; | ||
final Blob slice = browserBlob.slice( | ||
start ?? 0, | ||
end ?? browserBlob.size, | ||
browserBlob.type, | ||
); | ||
yield await blobToBytes(slice); | ||
} | ||
} | ||
|
||
/// Construct an [XFile] backed by [blob]. | ||
/// | ||
/// `name` needs to be passed from the outside, since it's only available | ||
/// while handling [html.File]s (when the ObjectUrl is created). | ||
class BlobXFile extends BaseBlobXFile { | ||
/// Construct an [XFile] backed by [blob]. | ||
/// | ||
/// `name` needs to be passed from the outside, since it's only available | ||
/// while handling [html.File]s (when the ObjectUrl is created). | ||
BlobXFile( | ||
Blob blob, { | ||
super.mimeType, | ||
super.displayName, | ||
super.lastModified, | ||
}) : _blob = blob; | ||
|
||
/// Creates a [XFile] from a web [File]. | ||
factory BlobXFile.fromFile(File file) { | ||
return BlobXFile( | ||
file, | ||
mimeType: file.type, | ||
displayName: file.name, | ||
lastModified: DateTime.fromMillisecondsSinceEpoch(file.lastModified), | ||
); | ||
} | ||
|
||
Blob _blob; | ||
|
||
// The Blob backing the file. | ||
@override | ||
Future<Blob> get blob async => _blob; | ||
|
||
/// Attempts to save the data of this [XFile], using the passed-in `blob`. | ||
/// | ||
/// The [path] variable is ignored. | ||
@override | ||
Future<void> saveTo(String path) async { | ||
// Save a Blob to file... | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This comment seems a bit redundant. |
||
await downloadBlob(_blob, name.isEmpty ? null : name); | ||
} | ||
} | ||
|
||
/// Constructs an [XFile] from the [objectUrl] of a [Blob]. | ||
/// | ||
/// Important: the Object URL of a blob must have been created by the same JS | ||
/// thread that is attempting to retrieve it. Otherwise, the blob will not be | ||
/// accessible. | ||
/// | ||
/// See: https://developer.mozilla.org/en-US/docs/Web/API/URL/createObjectURL_static | ||
class ObjectUrlBlobXFile extends BaseBlobXFile { | ||
/// Constructs an [XFile] from the [objectUrl] of a [Blob]. | ||
ObjectUrlBlobXFile( | ||
String objectUrl, { | ||
super.mimeType, | ||
super.displayName, | ||
super.lastModified, | ||
}) : _objectUrl = objectUrl; | ||
|
||
final String _objectUrl; | ||
|
||
Blob? _cachedBlob; | ||
|
||
// The Blob backing the file. | ||
@override | ||
Future<Blob> get blob async => _cachedBlob ??= await fetchBlob(_objectUrl); | ||
|
||
/// Returns the [objectUrl] used to create this instance. | ||
@override | ||
String get path => _objectUrl; | ||
|
||
/// Attempts to save the data of this [XFile], using the passed-in `objectUrl`. | ||
/// | ||
/// The [path] variable is ignored. | ||
@override | ||
Future<void> saveTo(String path) async { | ||
downloadObjectUrl(_objectUrl, name.isEmpty ? null : name); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
// Copyright 2013 The Flutter Authors. All rights reserved. | ||
// Use of this source code is governed by a BSD-style license that can be | ||
// found in the LICENSE file. | ||
|
||
import 'dart:io'; | ||
import 'dart:typed_data'; | ||
|
||
import 'base_bytes_x_file.dart'; | ||
|
||
/// A CrossFile backed by an [Uint8List]. | ||
class BytesXFile extends BaseBytesXFile { | ||
/// Construct an [XFile] from its data [bytes]. | ||
BytesXFile( | ||
super.bytes, { | ||
super.mimeType, | ||
super.displayName, | ||
super.lastModified, | ||
}); | ||
|
||
@override | ||
Future<void> saveTo(String path) async { | ||
final File fileToSave = File(path); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is lifted from the old implementation but... should the |
||
await fileToSave.writeAsBytes(bytes); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@navaronbracke thanks for the comment! This is a very interesting problem, for which I don't have a great answer. The TL;DR/Guidance is:
XFile
interface only.native/factory.dart
web/factory.dart
Each factory exposes constructors that are platform-specific, and we really don't have a cross-platform file constructor.
What problems do you think people are going to have? Is this a documentation problem, or a design issue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If people take the comment at face value and do
import "package:cross_file/web/factory.dart";
directly, they will run into errors likelibrary dart:js_interop is not available on this platform
. Since they don't directly importdart:js_interop
, this might be confusing to new users.I think this is merely a documentation issue. We should put emphasis on:
if (dart.library.js_interop) ...
and you probably need an extra abstraction, since the web-only types (i.e. Blob) cannot be imported in native implementations (for the same reason why the plugin cannot provide the conditional import here)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, if it's a documentation issue, it's probably easier to fix (LOL famous last words :P)
Do you have any ideas with how this comment would be clearer? What would you like to read?
(I'll try to come up with something more specific as well!)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, the main point is that the compiler itself would want you to use a conditional import.
So perhaps something like:
"In order to instantiate a new
XFile
, import the correct factory class, through a conditional import.Using a conditional import is required to use types that are only available on the current platform.
"
The wording is just a suggestion, so feel free to tweak it.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, wait. Since the API for both factories is divergent, would a user that uses this conditional import know at compile time if they could access the
fromFile()
(both with a dart:io or package:web File) or thefromBlob()/fromObjectUrl()
methods? I think that Dart will only provide the base typeXFileFactory
, but I think you don't get a guarantee for the available methods.Perhaps we can fix that by having the interface like:
XFileFactory
is the base type that is exported through a conditional export. It provides all the methods that do not include platform-specific types (so only fromPath/fromBytes/fromObjectUrl)WebXFileFactory
andNativeXFileFactory
implement that interfaceWebXFileFactory
provides thefromBlob/fromFile
implementations for package:web typesNativeXFileFactory
provides thefromFile
implementation for dart:ioUnsupportedError
kIsWeb
and/or a type check on the factoryThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the clarification @navaronbracke! I see what you mean! You want the
XFileFactory
to be cross-platform as well!IMO the only cross-platform class from this package is the
XFile
interface! 1If your cross-platform App needs to "create XFiles", you'll end up with some platform-aware code (because as you said, you can't import web stuff in native code), like this:
With a directory like this:
Your
get_file.dart
could conditionally export the right file for the platform; like:So each implementation file can use the platform-specific factories:
Maybe the confusing part is calling the
XFileFactory
the same on bothnative
andweb
? Should we just haveNativeXFileFactory
andWebXFileFactory
to make the separation clearer, and signal that we never expect a shared interface across both classes?This is why I think having a base class for the
XFileFactory
is more trouble than what it's worth. Unless I'm missing something, for this approach to work, theXFileFactory
needs to be an actualinstance
that must be replaced at run-time (similar to a federated plugin?).Since we can't expose web-only types through this common API, users would still need to conditionally import the native/web types to cast the instance to the one that has the platform specific methods. And if you're conditionally importing, just import the right
XFileFactory
static class? 😛Footnotes
This is similar to the
Client
interface frompackage:http
that has multiple platform-specific implementations coming from different packages ↩