Skip to content

Conversation

@corhere
Copy link
Contributor

Fixes#227.

The first version of Node where FS functions accepted an options object
was version 0.10. Between Node 0.10 and Node 6 inclusive, all falsy
values (including the empty string) and functions are replaced with the
default options. Only truthy values which are not strings, objects or
functions throw a TypeError.

Node 7 introduced a breaking change: only null, undefined and
functions get replaced with the default options, and any values which
are not strings or objects throw a TypeError. nodejs/node#7165

It is not clear which version of the Node FS API is emulated by
BrowserFS, so I have elected to implement the Node 7 behaviour.

Fixesjvilk#227. The first version of Node where FS functions accepted an options object was version 0.10. Between Node 0.10 and Node 6 inclusive, all falsy values (including the empty string) and functions are replaced with the default options. Only truthy values which are not strings, objects or functions throw a `TypeError`. Node 7 introduced a breaking change: only `null`, `undefined` and functions get replaced with the default options, and any values which are not strings or objects throw a `TypeError`. https://github.com/nodejs/node/pull/7165/files#diff-9a205ef7ee967ee32efee02e58b3482dR39 It is not clear which version of the Node FS API is emulated by BrowserFS, so I have elected to implement the Node 7 behaviour.
@jvilk
Copy link
Owner

jvilk commented Aug 22, 2018

The next major version of BrowserFS will clearly track a specific version of the NodeJS FS API. When starting this project (before Node hit 1.0), I had not anticipated that the fs API would continually evolve.

I'll merge this in for now, while acknowledging that there are likely other Node 7-incompatible behaviors.

@jvilkjvilk merged commit 87cb912 into jvilk:masterAug 22, 2018
Sign up for freeto 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

@corhere@jvilk