facebook / flow

Adds static typing to JavaScript to improve developer productivity and code quality.
https://flow.org/
MIT License
22.07k stars 1.85k forks source link

Can't use 100+ bytes literal types #5897

Closed aatatuzov closed 5 years ago

aatatuzov commented 6 years ago

It seems that Flow doesn't support variable-length string literal types. See this flow/try.

type Str100 = '0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789'
type Str99 = '123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789'

let str100: Str100 = '0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789'
let str99: Str99 = '123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789'
4: let str100: Str100 = '0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789'                        ^ Cannot assign `'0123456789...'` to `str100` because string [1] is incompatible with string literal `0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789` [2].
References:
4: let str100: Str100 = '0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789'                        ^ [1]
4: let str100: Str100 = '0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789'
               ^ [2]

At first glance you may think that such a long literal types do not make sense. However one (me) can use a JSON with phrase-based unicode fields for multilingual support. In this case Flow is supposed to check if there are i18n literals in code not defined in the JSON.

mrkev commented 6 years ago

Looked a bit into this: seems like there's a difference between a string of length 99 and one of length 100 in which the former gets parsed as Literal and the latter as AnyLiteral, which fails typechecking at https://github.com/facebook/flow/blob/master/src/typing/flow_js.ml#L2714

mrkev commented 6 years ago

Welp, nvm not a bug; it's right here. It's expensive to keep track of long strings so literals strings are capped at 100 characters.

Perhaps a way to set this constant this with a flag or through .flowconfig would come in handy for edge cases like yours. Unfortunately that is not on the roadmap, but if you'd like to PR I'd be happy to have a look at the branch.

aatatuzov commented 6 years ago

Thank you for response and information! I'll try my best to make PR with new .flowconfig option. I haven't done any PR nor tried OCaml yet but... it's never too late to start!

It's expensive to keep track of long strings so literals strings are capped at 100 characters.

Isn't this 100 bytes cap a temprorary fix? I mean if you gain some efficiency by truncating strings why wouldn't you replace them with short uids? I tried to find the answer by myself and found that commit with some ratinale about sharedmem and serialization but it doesn't say much.

mrkev commented 6 years ago

Using uids is a good idea and IMO definitely doable, but it involves potentially adding a lot of infrastructure (string -> uid conversion, then a uid -> string table for things like error reporting, passing the table around where necessary, etc) for what's admittedly a bit of an edge case. I'm definitely in favour of the idea but it probably won't see time investment anytime soon 😅

Having this arbitrary cutoff be configurable is IMO a good compromise. Thanks for the PR! I'll look into it 👍

aatatuzov commented 6 years ago

(string -> uid conversion, then a uid -> string table for things like error reporting, passing the table around where necessary, etc) for what's admittedly a bit of an edge case

I thought it may have a global optimization impact. Average literal length is at least two times greater than uid length. So one can probably save at least a half of a memory used for this purpose. Of course, I don't know if it really worth it.