TypeScript generate clients: this.baseUrl to cater for empty strings #3065
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hello!
First of all, thanks for all your fine work on NSwag. My team has happily made the journey from handrolling clients to generating TypeScript and C# clients for our platform using your marvellous tooling.
We bumped on one issue related which I raised in #2392.
The generated clients contain this line in the generated client code:
This code is problematic in the case where empty strings are supplied; this relates to JavaScript's
false
-y behaviour (where empty strings are treated asfalse
). Consider the following 2 cases:baseUrl
of"http://localhost:5000"
. - this is finebaseUrl
of an empty string:""
. - this has a problemThe first use case is fine. The second is problematic because of the false-y check. If
baseUrl
isnull
orundefined
then we fall back to use"http://fallbackUrl"
. That's desired behaviour. However, we also do the same for other falsey values including empty strings. That's troublesome!You can probably see where I'm going with this 😄
This PR changes the constructor in generated TypeScript clients to this:
This does the same as the previous code but crucially treats empty strings as legitimate values. This is a good thing ™️
The behaviour is similar to the nullish coalescing operator which is now part of JS / TS: microsoft/TypeScript#26578 / https://github.com/tc39/proposal-nullish-coalescing
Having this merged would remove our current workaround of doing some
string.Replace
on our generated TypeScript client code to get to this place.What do you think?
Thanks for all your work by the way - we really love it!
cc @pglx