typescript-jquery: Update to work with Typescript strict mode #3969

janjongboom · 2019-09-27T10:44:40Z

PR checklist

Read the contribution guidelines.
Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh, ./bin/openapi3/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows\. If contributing template-only or documentation-only changes which will change sample output, be sure to build the project first.
Filed the PR against the correct branch: master, 4.1.x, 5.0.x. Default: master.
Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

@TiFu @taxpon @sebastianhaas @kenisteward @Vrolijkx @macjohnny @nicokoenig @topce @akehir

Description of the PR

When compiling an API generated using the typescript-jquery generator against Typescript 3.6.2 in strict mode there are a number of issues with the generated code.

defaultExtraJQueryAjaxSettings is of type JQueryAjaxSettings | undefined but is set to null (valid in normal mode, not valid in strict mode).
In extendObj hasOwnProperty is called on objB. But objB is not guaranteed to be an object as it inherits from unknown type T1.
Properties in the Configuration class are marked as string, but have no initializer, thus giving false safety when consuming this class (values could be undefined too).

This PR adds some small guards around these issues.

macjohnny

Thanks for the PR

macjohnny · 2019-09-27T10:55:54Z

samples/client/petstore/typescript-jquery/default/api/PetApi.ts

     */
-    public addPet(body: models.Pet, extraJQueryAjaxSettings?: JQueryAjaxSettings): JQuery.Promise<
+    public addPet(pet: models.Pet, extraJQueryAjaxSettings?: JQueryAjaxSettings): JQuery.Promise<


@janjongboom please run the https://github.com/OpenAPITools/openapi-generator/blob/master/bin/typescript-jquery-petstore-all.sh script, not the openapi3 version.

macjohnny · 2019-09-27T10:56:08Z

samples/client/petstore/typescript-jquery/default/api/PetApi.ts

@@ -39,7 +39,7 @@ export class PetApi {

    private extendObj<T1, T2 extends T1>(objA: T2, objB: T2): T1|T2 {
        for (let key in objB) {
-            if (objB.hasOwnProperty(key)) {
+            if (objB[key]) {


where does this change come from?

Hmm... That should not be there, something went wrong with rebasing. Will repush.

janjongboom · 2019-09-28T14:32:57Z

@macjohnny Pushed changes as requested.

macjohnny · 2019-09-28T15:48:39Z

samples/client/petstore/typescript-jquery/default/model/InlineObject.ts

@@ -0,0 +1,26 @@
+/**
+ * OpenAPI Petstore


Please delete this file

macjohnny · 2019-09-28T15:49:04Z

samples/client/petstore/typescript-jquery/npm/model/InlineObject.ts

+
+export interface InlineObject {
+    /**
+     * Updated name of the pet


Please delete this file

macjohnny · 2019-09-28T15:49:19Z

samples/client/petstore/typescript-jquery/default/model/InlineObject1.ts

+
+import * as models from './models';
+
+export interface InlineObject1 {


Please delete this file

@macjohnny Removed and re-pushed. However, these were generated by the tool, so interesting to see where they'd come from.

these files are generated with the bin/openapi3/typescript-fetch-petstore-all.sh command. the output is supposed to be the same as with bin/typescript-fetch-petstore-all.sh, but it isn't, so it needs to be addressed in a separate issue.

CI currently checks against the output of bin/typescript-fetch-petstore-all.sh

janjongboom · 2019-10-21T10:26:19Z

Rebased and pushed to restart tests.

macjohnny · 2019-10-21T10:34:37Z

samples/client/petstore/typescript-jquery/npm/model/InlineObject1.ts

@@ -0,0 +1,26 @@
+/**


please delete this file

@macjohnny I'm not sure where these come from, but the file is referenced in https://github.com/OpenAPITools/openapi-generator/pull/3969/files/c99fcd7b5936800b4ef0d38e92ebd27dc86fadcb#diff-969fc1a4f92fd269b7aac842f2049bb7R15 as well. This is just the output of the build step.

macjohnny · 2019-10-21T10:35:00Z

samples/client/petstore/typescript-node/default/api/petApi.ts

     */
-    public async addPet (body: Pet, options: {headers: {[name: string]: string}} = {headers: {}}) : Promise<{ response: http.ClientResponse; body?: any;  }> {


this is unrelated

Similar to above. Apparently something was landed earlier but the samples weren't updated (generated by the build script). I'm happy to revert this file back to master but shouldn't samples always be reflective of the latest version of the tool?

I will update your branch with the correct samples

macjohnny · 2019-10-21T11:03:47Z

@janjongboom I cleaned the samples in your branch. By the way: no need to re-base, PRs are squashed.

janjongboom · 2019-10-21T11:10:29Z

@macjohnny OK, thanks!

wing328 · 2019-10-31T09:26:29Z

@janjongboom thanks for the PR, which has been included in v4.2.0 release: https://twitter.com/oas_generator/status/1189824932345069569

janjongboom force-pushed the jquery-ts3.6.2 branch from f74aea4 to e98a90e Compare September 27, 2019 10:44

macjohnny reviewed Sep 27, 2019

View reviewed changes

macjohnny added this to the 4.1.3 milestone Sep 27, 2019

macjohnny added the Client: TypeScript label Sep 27, 2019

janjongboom force-pushed the jquery-ts3.6.2 branch from e98a90e to ff96892 Compare September 28, 2019 14:32

macjohnny requested changes Oct 3, 2019

View reviewed changes

wing328 modified the milestones: 4.1.3, 4.2.0 Oct 4, 2019

janjongboom force-pushed the jquery-ts3.6.2 branch from ff96892 to 06ec44b Compare October 9, 2019 08:20

typescript-jquery: Work in Typescript strict mode

c99fcd7

janjongboom force-pushed the jquery-ts3.6.2 branch from 06ec44b to c99fcd7 Compare October 21, 2019 10:26

macjohnny requested changes Oct 21, 2019

View reviewed changes

re-generate samples

c78837f

macjohnny approved these changes Oct 21, 2019

View reviewed changes

macjohnny merged commit 1052eba into OpenAPITools:master Oct 21, 2019


		import * as models from './models';

		export interface InlineObject1 {

		*/
		public async addPet (body: Pet, options: {headers: {[name: string]: string}} = {headers: {}}) : Promise<{ response: http.ClientResponse; body?: any; }> {

Uh oh!

typescript-jquery: Update to work with Typescript strict mode #3969

typescript-jquery: Update to work with Typescript strict mode #3969

Uh oh!

Conversation

janjongboom commented Sep 27, 2019

PR checklist

Description of the PR

Uh oh!

macjohnny left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

janjongboom commented Sep 28, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

janjongboom commented Oct 21, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

macjohnny commented Oct 21, 2019

Uh oh!

janjongboom commented Oct 21, 2019

Uh oh!

wing328 commented Oct 31, 2019

Uh oh!

Uh oh!