fs.readFile() can return string?

[bolinfest]

It appears fs.readFile() can return a string instead of a Buffer if an encoding option is specified, such as:

fs.readFile(json, {encoding: 'UTF-8'}, function(data) { JSON.parse(data); });

As such, I think the definition needs to be changed to:

  declare function readFile(
    filename: string,
    options?: Object | string,
    callback?: (err: ?Error, data: Buffer | string) => void
  ): void;

[gyzerok]

Are you sure that it is not an implicit typecast? Node.js docs aren't clear for this.

[bolinfest]

Yes: I checked with typeof. On Apr 27, 2015 2:17 AM, "Fedor Nezhivoy" notifications@github.com wrote:

Are you sure that it is not an implicit type cast?

— Reply to this email directly or view it on GitHub https://github.com/facebook/flow/issues/416#issuecomment-96580126.

[gyzerok]

@bolinfest ok, thanks. Suppose readFileSync behaves similarly?

[samwgoldman]

We talked about this problem on IRC, and the Buffer | string type doesn't really capture the actual type signature very elegantly. In particular, even if you know you provided an encoding or not, callers would still need to refine the type of data in order to use string-only or Buffer-only properties, so lots of existing, untyped code will throw errors.

Ideally, we could declare this function to properly express the dependency between the options type and the return type. Something like:

declare function readFile(
  filename: string,
  options: { encoding: string; flag?: string },
  callback: (err: ?Error, data: string) => void
)
declare function readFile(
  filename: string,
  options?: { flag?: string },
  callback: (err: ?Error, data: Buffer) => void
)

For now, I think the best course of action is to type data as any. This way, existing code will not cause type errors. We should spend time making overloaded declarations work as expected.

/cc @gabelevi

[dead-claudia]

I would say the best fix is to allow for method signature overloading in method declarations.

[sergey-lapin]

+1 for fixing this bug, I guess that usage in this form fs.readFile(filename, 'utf-8') is even more popular than without encoding.

[dead-claudia]

Eh... Apparently so.

On Tue, Jun 16, 2015, 13:38 Sergey Lapin notifications@github.com wrote:

+1 for fixing this bug, I guess that usage in this form fs.readFile(filename, 'utf-8') is even more popular than without encoding.

— Reply to this email directly or view it on GitHub https://github.com/facebook/flow/issues/416#issuecomment-112509104.

[alandotcom]

Is anyone actively working on this?

[samwgoldman]

@lumberj #653

[samwgoldman]

Fixed in 0.14