mlly icon indicating copy to clipboard operation
mlly copied to clipboard

fix: mark `names` field in `ESMExport` as possibly undefined

Open tobiasdiez opened this issue 1 year ago • 3 comments

The names field in ESMExport could be undefined as (indirectly) has been reported in https://github.com/nuxt/module-builder/issues/309.

This is now properly reflected in the types. Moreover, the regex group matches are now typed correctly.

tobiasdiez avatar Aug 11 '24 23:08 tobiasdiez

Thanks for PR and sorry for late reply. I love the internal type util for regexes!

However, considering the types already declare names as existing, i think we might normalize it to an empty array inside normalizeExports wdyt?

pi0 avatar Oct 06 '24 20:10 pi0

Thanks for PR and sorry for late reply. I love the internal type util for regexes!

No worries!

However, considering the types already declare names as existing, i think we might normalize it to an empty array inside normalizeExports wdyt?

I don't have a strong opinion on this. I guess you can make a case for both: returning an empty array would be slightly easier to use, while making names optional aligns more with the other optional fields and may better convey that the result is not always available / makes sense.

Let me know what you prefer and I'll implement it.

tobiasdiez avatar Oct 07 '24 02:10 tobiasdiez

I think mainly for the sake of not breaking current deps (that assume names is not nullable based on types) it would be a faster solution to set an empty array.

I agree about benefits of making it optional, perhaps we can do it in next major versions.

pi0 avatar Oct 07 '24 08:10 pi0