Kotlin / dukat

Converter of <any kind of declarations> to Kotlin external declarations
552 stars 44 forks source link

Multiple index.d.ts in translation cause file overwriting with data loss #382

Open trilis opened 3 years ago

trilis commented 3 years ago

Sometimes, we can observe in logs, that multiple files with the same name were created, like this:

index.module_definitely-typed.kt
index.module_definitely-typed.kt
index.Validation.module_definitely-typed.kt

Of course, only the latest of these files will be saved, so there is data loss of previous versions.

Looks like this bug is reproduced every time we have multiple index.d.ts files in different folders (although I'm not sure about "every time" part).

List of DT libraries on which I reproduced this behaviour:

set-interval-async
storybook-readme/html
oracle__oraclejet/ojvalidation-datetime
oracle__oraclejet/ojknockout-keyset
oracle__oraclejet/ojvalidation-number
cometd/BinaryExtension
cometd/AckExtension
cometd/TimeSyncExtension
wordpress__block-editor/utils
react-select/v1/lib/utils/defaultMenuRenderer
Schahen commented 3 years ago

Marking as testing because in 99.9% percent of cases this affects our pipeline, not real-life scenarios (it's not about code installed via npm install but rather about reading tsonfig.json)

trilis commented 3 years ago

Also observed following tricky case:

Observable.module_rxjs.kt
observable.module_rxjs.kt

which is formally two strings, but on most of OS this will result in rewrite (I suppose, this case can be reproduced in non-test environment as well)

shabunc commented 3 years ago

it's strange, since to my knowledge dukat supposed to take care of such cases, for instance this:

declare namespace Oberon {
    class Something { }
}

declare namespace oberon {
    interface Anything {}
}

generates two files: a.Oberon.module_tmp.kt and a.oberon.module_tmp._2.kt

trilis commented 3 years ago

Can you test this on declarations from separate modules? I'm suggesting this because code here looks like it has separate clash maps for each module (which is serious bug, if true)

trilis commented 3 years ago

You can create two folders with index.d.ts in each, and then launch dukat like this:

dukat f1/index.d.ts f2/index.d.ts

Output:

index.kt
index.kt

(containing only declarations from f2/index.d.ts) I don't think this is test-only issue anymore.