Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

TypeScript generate clients: this.baseUrl to cater for empty strings #3065

Merged
merged 1 commit into from
Sep 28, 2020
Merged

TypeScript generate clients: this.baseUrl to cater for empty strings #3065

merged 1 commit into from
Sep 28, 2020

Conversation

johnnyreilly
Copy link
Contributor

@johnnyreilly johnnyreilly commented Sep 19, 2020

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.baseUrl = baseUrl ? baseUrl : "http://fallbackUrl";

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 as false). Consider the following 2 cases:

  1. When developing locally we pass a baseUrl of "http://localhost:5000". - this is fine
// Development mode - the supplied baseUrl is "http://localhost:5000"  
this.baseUrl = baseUrl ? baseUrl : "http://fallbackUrl"; //"http://localhost:5000" - yay!
  1. When deployed, our complete app: API, HTML, CSS and JavaScript generated from TypeScript all get served from the same location. So in that case we pass a baseUrl of an empty string: "". - this has a problem
// Production mode - the supplied baseUrl is ""  
this.baseUrl = baseUrl ? baseUrl : "http://fallbackUrl"; //"http://fallbackUrl" - oh no! we wanted to use the empty string; that was the behaviour we needed!

The first use case is fine. The second is problematic because of the false-y check. If baseUrl is null or undefined 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.baseUrl = baseUrl !== undefined && baseUrl !== null ? baseUrl : "http://fallbackUrl";

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

@johnnyreilly johnnyreilly changed the title this.baseUrl to cater for empty strings #2392 TypeScript generate clients: this.baseUrl to cater for empty strings Sep 19, 2020
@RicoSuter RicoSuter merged commit 79108da into RicoSuter:master Sep 28, 2020
@RicoSuter
Copy link
Owner

Makes sense, I hope we dont break someone with this...
Thanks for the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants