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

feat: fix missing createWritable in safari #62

Merged
merged 3 commits into from
Jul 19, 2023
Merged

feat: fix missing createWritable in safari #62

merged 3 commits into from
Jul 19, 2023

Conversation

jimmywarting
Copy link
Owner

@jimmywarting jimmywarting commented Jun 28, 2023

Safari do have a OPFS but it's lacking any good method of writing things to it unless you are not using the syncAccessHandle that's only available in web workers.

This will patch/repair/polyfill the missing features in the broken api.

  • FileSystemWritableFileStream is added to global scope only if it dose not exist but a File system dose.
  • FileSystemFileHandle.prototype.createWritable is conditionally polyfilled if needed.
    • note that this will also patch getFileHandle to keep track of what the parent directory is as createWritable needs to know the path of where it's located in order to re-create the FileSystemFile handle inside of a worker ( cuz they can't be structural cloned / postMessaged )
  • Safari did also lacked some assertion test to see if the file/folder still existed before doing some kind of operation on them. so some methods have been patched to ensure that they do still exist

@Mazuh
Copy link

Mazuh commented Nov 21, 2023

@jimmywarting any estimate of when you'll publish a stable version on NPM including this fix?

@jimmywarting
Copy link
Owner Author

oh... did i forget to publish?

sry about that. i have nothing else in the work that needs to be completed. expect a new release soon-ish

@SargisPlusPlus SargisPlusPlus mentioned this pull request Feb 8, 2024
@SargisPlusPlus
Copy link
Contributor

SargisPlusPlus commented Feb 8, 2024

@jimmywarting I had an issue with this... I published the library myself to test out this code.
In my react app, I have:

          const opfsRoot = await getOriginPrivateDirectory();
          const userDir = await opfsRoot.getDirectoryHandle('123', {
            create: true,
          });
           const fileHandle = await userDir.getFileHandle('test.txt', {
              create: true,
           });
          try {
            const writable = await fileHandle.createWritable();
          } catch (e) {
            console.log('****', e);
          }

This works in Chrome. But Safari (17.2) throws an error: **** NotFoundError: A requested file or directory could not be found at the time an operation was processed.

UPDATE: I think I may have found the issue. See comments.

// So we need to pass the path to the worker. This is a bit hacky and ugly.
const root = await navigator.storage.getDirectory()
const parent = await wm.get(this)
const path = await parent.resolve(root)
Copy link
Contributor

@SargisPlusPlus SargisPlusPlus Feb 8, 2024

Choose a reason for hiding this comment

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

@jimmywarting path was null for me. I had to swap these two.

const path = await root.resolve(parent)

globalThis.FileSystemFileHandle.prototype.createWritable = async function (options) {
// Safari only support writing data in a worker with sync access handle.
if (!workerUrl) {
const blob = new Blob([code.toString() + `;${code.name}();`], {
Copy link
Contributor

@SargisPlusPlus SargisPlusPlus Feb 8, 2024

Choose a reason for hiding this comment

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

another issue here...
not sure why but

code.toString() + `;${code.name}();`

produces:
function() {....}
which throws a syntax error because function doesnt have name.

To solve this, i change code to be arrow function:

  const code = () => { ... }

and then here I have:

      const stringCode = `(${code.toString()})()`;
      const blob = new Blob([stringCode], {

which produces:

(() => {...})()

And this works.

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.

3 participants