(@fluent/dedent) Migrate to TypeScript#451
Conversation
fluent-dedent/src/index.ts
Outdated
| function dedent(line, idx) { | ||
| let lineIndent = line.slice(0, commonIndent.length); | ||
| function dedent(line: string, idx: number): string { | ||
| let lineIndent = line.slice(0, (commonIndent as string).length); |
There was a problem hiding this comment.
TS needs a little help here; otherwise it considers commonIndent as string | undefined. Even though there's the commonIndent === undefined check above, the reference here is in a closure and could possibly be used somewhere outside.
An alternative would be to create a new binding just for the closure, like so:
let commonIndent = lines.pop();
if (commonIndent === undefined || !RE_BLANK.test(commonIndent)) {
throw new RangeError("Closing delimiter must appear on a new line.");
}
// commonIndent is inferred and guaranteed to be a string.
let commonLength = commonIndent.length;
function dedent(line: string, idx: number): string {
let lineIndent = line.slice(0, (commonIndent as string).length);
// ...
}
return lines.map(dedent).join("\n");I thiknk this reads quite well too, but the drawback is that this solution pollutes and compiled JS. OTOH, the commonIndent as string assertion is TS-only and is removed during compilation.
There was a problem hiding this comment.
I did a quick check on this in the playground and it seems declaring commonIndent as a const and writing dedent as an arrow function (so the function won't be hoisted) should allow the commonIndent === undefined type guard to work correctly.
There was a problem hiding this comment.
You're right, thank you! I implemented your suggestion, and it made me think that just getting rid of the closure would also be a solution :) So I ended up doing that instead.
Pike
left a comment
There was a problem hiding this comment.
Sorry for the lag. I think this is OK. There's a bunch of magic typing here, but if that works for you, I don't mind.
See #376.