Closed ZeeD closed 1 year ago
What is your expected outcome in this situation? Would you prefer to keep the first file written to disk, or the last file written to disk? I guess I would go with the last file personally, because that would allow plugins to replace files during transpile (even though that is probably a result of some bug in the plugin implementation), but I think rooibos writes files in the beforeFileTranspile
event hook if I remember correctly.
Honestly I don't know: for what I know this could really be just an unfortunate accident, and the two different files could point to the same content. I would just point out that I think that the current implementation check doesn't work as expected, as working with set of objects is "fragile" (I have written a simple demo on https://jsbin.com/sarizatibo/1/edit?js,console)
I don't know so well the internals of rooibos
, but maybe the current logic to keep the first element is good enough.
If you want I can try to save both versions of the file and check if they are different, and what to keep
So, one of the reasons for using Set
is because, every time the contents of a file changes, we destroy the current file instance and create a new instance of the class. While this doesn't mitigate the "there's already a file here so don't overwrite" logic, it DOES handle the situation where the same file may have changed but had its output path pointed somewhere else.
However, that situation would also be handled by utilizing the getOutputPath
functionality you suggested above, so it's probably a moot point.
Feel free to submit a pull request that converts the transpile logic to utilize the output path instead. (make sure to call outputPath.toLowerCase()
before adding to and checking the set).
Feel free to submit a pull request that converts the transpile logic to utilize the output path instead. (make sure to call
outputPath.toLowerCase()
before adding to and checking the set).
Is roku case insesitive for the filenames? I didn't knew. (or it is for some additional precaution?)
Yep, the Roku file system is case insensitive.
fixed with #691
(this is a bit contrived, and if you prefer I'll try to write a proper PR)
I think that there is a bug in
https://github.com/rokucommunity/brighterscript/blob/33f83183374c483d6e2af3a8479af0fcfd1b9b09/src/Program.ts#L1391
specifically: as
processedFiles
is aSet
ofFile
(so objects, so on), and it's filled with files onhttps://github.com/rokucommunity/brighterscript/blob/33f83183374c483d6e2af3a8479af0fcfd1b9b09/src/Program.ts#L1397
and then checked on
https://github.com/rokucommunity/brighterscript/blob/33f83183374c483d6e2af3a8479af0fcfd1b9b09/src/Program.ts#L1440
the check fails if there is a "new file" that just happens to point to an already created
outputPath
.I think it would be better to switch to a
Set<string>
(where the values are theoutputPath
s) so to haveI found that using a
rooibos
to run unit test on my library, if I try to run a test suite tagged with@sgnode("a-component")
the plugin tries to autogenerate some .xml files, butbsc
ended with(BTW: thanks for the error message! I would have been lost without it)
sidenote: according to mdn sets only support value equality, meaning that, for objects only strict equality is supported meaning that they should be the same object witch IMHO limit A LOT the usefulness of the
Set
itself, as a data structure :(