Nswag: baseUrl not set correctly for TypeScript with version 13.8.0.0

Created on 28 Sep 2020  路  17Comments  路  Source: RicoSuter/NSwag

We have a project which generates TS code using NSwag and today I updated to version 13.8.0.0, and it seems to have broken how baseUrl is set.

Where it previously, in the constructor, generated:

this.baseUrl = baseUrl ? baseUrl : this.getBaseUrl("");

It now generates:

this.baseUrl !== undefined && baseUrl !== null ? baseUrl : this.getBaseUrl("");

Notice the value isn't actually set anywhere.

A quick search/replace to the following, fixed the problem:

this.baseUrl = baseUrl !== undefined && baseUrl !== null ? baseUrl : this.getBaseUrl("");

I have made other changes to my code recently, but nothing I can think of would cause this other than upgrading the packages.

I'll gladly provide more info, if you let me know what you need to know. I was just wondering, if what you were after, was:

this.baseUrl = baseUrl ?? this.getBaseUrl("");

All 17 comments

I guess this PR is to blame ... #3065

Cc @johnnyreilly

Cant you inject null or undefined or disable injecting base url at all (i think there is a config for that)?

Are you asking me or @johnnyreilly ?

As far as I can tell, this.baseUrl = baseUrl ?? this.getBaseUrl(""); works as expected - not falsey for emtpy strings.

'' ? '' : 'something' will give you 'something, but '' ?? 'something' gives you ''.

I鈥檓 asking you @CRidge

I wonder what the difference with ?? is to what we have now?

@RicoSuter The problem with what is there right now, is that the result isn't assigned to the expected property :) The difference with pre 13.8.0 is that ? uses falsey logic (0, false or '' = false), where as ?? uses nullish logic (null or undefined = null).

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Nullish_coalescing_operator

So if the result of what is there now is actually assigned, it would be the same thing as with ?? - just a lot more code than needed.

@CRidge how do set a baseUrl in your case? I think I'm not quite following what you're saying - are you saying that NSwag now generates:

this.baseUrl = baseUrl !== undefined && baseUrl !== null ? baseUrl : this.getBaseUrl("");

This would seem to be correct? Or is generating something different? My apologies if there's a bug in my PR - I'm happy to submit a fix if that's the case.

However this would seem to be the desired (nullish) behaviour I would think?

this.baseUrl = baseUrl !== undefined && baseUrl !== null ? baseUrl : this.getBaseUrl("");

Actually I think I see a problem https://github.com/RicoSuter/NSwag/pull/3065/commits/fa43fdfd1f1e2f0581c5cc64dd638038bc7ee07f

Give me 10 minutes and I'll raise a fix PR

Fix PR raised: https://github.com/RicoSuter/NSwag/pull/3079 - my apologies for the problems. :sunflower:

Oh shit, i missed that.

So if the result of what is there now is actually assigned, it would be the same thing as with ?? - just a lot more code than needed.

I agree but NSwag/NJS supports (very) old TS versions where ?? is not available - so it's safer to use ? instead which is then hopefully minimized/optimized anyway. Is that ok?

v13.8.1 with quick fix - should be ready soon.

OK, for backwards compatibility - no problem. Looking forward to 13.8.1 馃憤

I've modified the auto-generated TS to match the fix but I get this error:

Type 'string | undefined' is not assignable to type 'string'.
Type 'undefined' is not assignable to type 'string'.  TS2322

constructor(baseUrl?: string, http?: { fetch(url: RequestInfo, init?: RequestInit): Promise<Response> }) {
    this.http = http ? http : <any>window;
    this.baseUrl = this.baseUrl !== undefined && baseUrl !== null ? baseUrl : "";
    ^
}

Using Nullish Coalescing solves the issue:

constructor(baseUrl?: string, http?: { fetch(url: RequestInfo, init?: RequestInit): Promise<Response> }) {
    this.http = http ? http : <any>window;
    this.baseUrl = baseUrl ?? "";
}

https://www.typescriptlang.org/docs/handbook/release-notes/typescript-3-7.html#nullish-coalescing

Ahh, we need to cast to any to avoid this? 馃檪

Sorry, I'm learning TypeScript as I debug...
I had a "this." in the condition.
All good. 13.8.1 fixes it. 馃檪

Thank you @johnnyreilly for your quick fix! 馃槈

I agree but NSwag/NJS supports (very) old TS versions where ?? is not available - so it's safer to use ? instead which is then hopefully minimized/optimized anyway. Is that ok?

Yeah this was my rationale for not using the nullish coalescing operator. Not everyone is on the latest and greatest TypeScript - and in fact I think support for that only shipped with TypeScript 3.8 so it's pretty new.

It's worth saying that most people will be transpiling to support older browsers which don't support nullish coalescing. So even if we directly used the nullish coalescing operator, most users would actually be executing something like our generated code anyway 鈽猴笍

Thanks for the quick fix!

Was this page helpful?
0 / 5 - 0 ratings