-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Modifying buffer output in onEnd callback doesn't update text output #2423
Comments
This is not a feature. There is nothing in the documentation that says this is supposed to work. I recommend calling |
I agree this not documented that it should work as simply as this, but this is what I expected when reading Would you accept a PR for:
function convertOutputFiles({ path, contents }: protocol.BuildOutputFile): types.OutputFile {
let text: string | null = null;
let mutableContents = contents;
return {
path,
get contents() {
return mutableContents;
},
set contents(value) {
mutableContents = value;
text = null
},
get text() {
if (text === null) text = protocol.decodeUTF8(mutableContents);
return text;
},
set text(value) {
text = value;
mutableContents = protocol.encodeUTF8(value)
},
}
} |
Thanks for the change. I understand the performance reason for not having a setter for text, but this not obvious at first that this is only a getter. This could be made clearer by adding a small precision in the doc like: |
Changing the type of |
Having a TS error because some part of your code didn't make sense is not a breaking change IMO because it doesn't break you code (.i.e nothing change at runtime) and you can just add an ts-ignore comment. But I understand that for a project of that size there is always people complaining |
Following #1792
Repro
version:
0.14.51
Result
Logs
const a = 2; const a = 1;
Expected result
Logs
const a = 2; const a = 2;
Workaround
Uncommenting the defineProperty line gives the expected result.
The text was updated successfully, but these errors were encountered: