Add Deno support to typescript(experimental) generator by chibat · Pull Request #6732 · OpenAPITools/openapi-generator

chibat · 2020-06-21T12:04:49Z

This PR add Deno support to typescript(experimental) generator.
closes #3174

PR checklist

Read the contribution guidelines.
If contributing template-only or documentation-only changes which will change sample output, build the project beforehand.
Run the shell script ./bin/generate-samples.shto update all Petstore samples related to your fix. This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master. These must match the expectations made by your contribution. You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/config/java*. For Windows users, please run the script in Git BASH.
File the PR against the correct branch: master
Copy the technical committee to review the pull request if your PR is targeting a particular programming language.

@TiFu (2017/07) @taxpon (2017/07) @sebastianhaas (2017/07) @kenisteward (2017/07) @Vrolijkx (2017/09) @macjohnny (2018/01) @topce (2018/10) @akehir (2019/07) @petejohansonxo (2019/11) @amakhrov (2020/02)

TiFu

Thanks for the PR! I have some questions and potentially one small change request - see my comments in the code.

...napi-generator/src/main/java/org/openapitools/codegen/languages/TypeScriptClientCodegen.java

TiFu · 2020-06-21T12:38:53Z

modules/openapi-generator/src/main/resources/typescript/http/http.mustache

    private headers: { [key: string]: string } = {};
    private body: RequestBody = undefined;
-    private url: URLParse;
+    private url: {{#platforms}}{{#deno}}URL{{/deno}}{{^deno}}URLParse{{/deno}}{{/platforms}};


Could we instead create a separate module that wraps deno's URL and provides a URLParse like interface?

We could then import this module instead of adding all of these conditions.

I understand. I try it.

Why do we use URLParse (external dependency) at all? Node provides whatwg-compatible URL (https://nodejs.org/docs/latest-v12.x/api/url.html#url_class_url), as well as modern browsers.

We are using url-parse to support IE ?
If we want to support IE, is it better to use URLSearchParams Polyfill ?

I think we had this discussion in one of the PRs of the original generator. I think the decision was based on url-parse being easier to use compared to the polyfill. url-parse also only has two dependencies (including recursive dependencies).

Open to replacing url-parse in the browser as long as we hide it behind an interface. @bodograumann similarly added a workaround for uploading files in IE.

@amakhrov I absolutely agree! @TiFu would you mind removing the polyfills before the 5.0.0 release?

The question for me is, where do we draw the line?
We could use Blob.text(), as it is a WebAPI, but it is not supported in lots of browsers.
Then there is URLSearchParams.
And of course fetch API.

When someone generates the code, they don't know which polyfills they need to get it running in a certain set of browsers. In my experience babel does not automatically add polyfills for (most of) these things.

I think it is a complex issue and maybe you can open a separate issue, @amakhrov. I would love to discuss the details.

This time, I have created URLParse wrapper for Deno.
Type of URLParse is complex..
I have replaced instantiating from a function call to new.

I agree with @bodograumann - let's discuss how we want to proceed. I'd prefer something that just works out of the box in the targeted environment - be it node or browsers or deno.

Users must have a choice. Right now we force them to depend on a specific polyfill, regardless on the platform they target

TiFu · 2020-06-21T16:00:59Z

Could you also please run ./bin/utils/export_docs_generator.sh to update the documentation of the generator? (that's what circleci is complaining about)

amakhrov · 2020-06-22T05:56:36Z

modules/openapi-generator/src/main/resources/typescript/index.mustache

@@ -1,41 +1,45 @@
+{{#platforms}}
+{{^deno}}
 import "es6-promise/auto";


Also curious why we enforce this polyfill instead of delegating that to the end consumer (which might already have a different polyfill in their app!)

It's not a comment about this PR, though - it's actually about the typescript generator already in master :)

I remove it and try running the integration test.

amakhrov · 2020-06-22T05:57:30Z

modules/openapi-generator/src/main/resources/typescript/auth/auth.mustache

-import { RequestContext } from "../http/http";
+import { RequestContext } from "../http/http{{#platforms}}{{#deno}}.ts{{/deno}}{{/platforms}}";
 {{#useInversify}}
 import { injectable, inject, named } from "inversify";


looks like there is no intention to support inversify on the Deno platform - is that so?

Yes. Inversify may work on Deno. But I think it's probably not easy.

Why would it be difficult to add?
If we can't support inversify on deno, could you please document that in the inversify option?

I have checked that if we can use Inversify on Deno.

Deno Third Party Modules

Inversify is not listed in Deno Third Party Modules.

Pika

https://www.pika.dev/search?q=inversify
message: "Package found! However, no web-optimized "module" entrypoint was found in its package.json manifest."

jspm

$ cat inversify_sample.ts import "https://jspm.dev/reflect-metadata"; // @deno-types="https://cdn.jsdelivr.net/npm/inversify@5.0.1/dts/inversify.d.ts" import { Container, injectable } from "https://jspm.dev/inversify@5.0.1"; var container = new Container(); $ ~/data/work$ deno run --reload -c tsconfig.json inversify_sample.ts Download https://jspm.dev/reflect-metadata Download https://jspm.dev/inversify@5.0.1 Download https://cdn.jsdelivr.net/npm/inversify@5.0.1/dts/inversify.d.ts Download https://jspm.dev/npm:inversify@5.0.1/_/2b7febed.js Download https://jspm.dev/npm:inversify@5.0.1/_/2e4eea2e.js Download https://jspm.dev/npm:inversify@5.0.1/_/d819b7b6.js Download https://jspm.dev/npm:inversify@5.0.1/_/8b88d942.js Download https://jspm.dev/npm:inversify@5.0.1!cjs Download https://cdn.jsdelivr.net/npm/inversify@5.0.1/dts/constants/metadata_keys Download https://cdn.jsdelivr.net/npm/inversify@5.0.1/dts/container/container Download https://cdn.jsdelivr.net/npm/inversify@5.0.1/dts/constants/literal_types Download https://cdn.jsdelivr.net/npm/inversify@5.0.1/dts/container/container_module Download https://cdn.jsdelivr.net/npm/inversify@5.0.1/dts/annotation/injectable Download https://cdn.jsdelivr.net/npm/inversify@5.0.1/dts/annotation/tagged Download https://cdn.jsdelivr.net/npm/inversify@5.0.1/dts/annotation/named Download https://cdn.jsdelivr.net/npm/inversify@5.0.1/dts/annotation/inject Download https://cdn.jsdelivr.net/npm/inversify@5.0.1/dts/annotation/optional Download https://cdn.jsdelivr.net/npm/inversify@5.0.1/dts/annotation/unmanaged Download https://cdn.jsdelivr.net/npm/inversify@5.0.1/dts/annotation/multi_inject Download https://cdn.jsdelivr.net/npm/inversify@5.0.1/dts/annotation/target_name Download https://cdn.jsdelivr.net/npm/inversify@5.0.1/dts/annotation/post_construct Download https://cdn.jsdelivr.net/npm/inversify@5.0.1/dts/planning/metadata_reader Download https://cdn.jsdelivr.net/npm/inversify@5.0.1/dts/utils/id Download https://cdn.jsdelivr.net/npm/inversify@5.0.1/dts/interfaces/interfaces Download https://cdn.jsdelivr.net/npm/inversify@5.0.1/dts/annotation/decorator_utils Download https://cdn.jsdelivr.net/npm/inversify@5.0.1/dts/syntax/constraint_helpers Download https://cdn.jsdelivr.net/npm/inversify@5.0.1/dts/utils/serialization Download https://cdn.jsdelivr.net/npm/inversify@5.0.1/dts/utils/binding_utils Download https://jspm.dev/npm:@jspm/core@2/nodelibs/process Download https://jspm.dev/npm:reflect-metadata@0.1.13!cjs Download https://jspm.dev/npm:@jspm/core@2.0.0-beta.7/_/de9a8dfb.js error: Import 'https://cdn.jsdelivr.net/npm/inversify@5.0.1/dts/utils/binding_utils' failed: 404 Not Found Imported from "https://cdn.jsdelivr.net/npm/inversify@5.0.1/dts/inversify.d.ts:21" $ echo $? 1

Inversify may work on Deno via jspm. But I couldn't load the Type Definition file.
Inversify may officially support Deno in the future.
At the moment, I think it's better not to support by openapi-generator yet.
This time, I added a description to the documentation that it is not currently supported.

amakhrov · 2020-06-22T06:01:14Z

modules/openapi-generator/src/main/resources/typescript/http/http.mustache

 {{#frameworks}}
 {{#fetch-api}}
-export * from './isomorphic-fetch';
+export * from './isomorphic-fetch{{#platforms}}{{#deno}}.ts{{/deno}}{{/platforms}}';


this change looks redundant, since this whole line is not generated for Deno (which uses a built-in fetch anyway)

Thanks! I fixed.

modules/openapi-generator/src/main/resources/typescript/api/api.mustache

macjohnny · 2020-06-24T09:02:14Z

anyone against merging this so it will be part of the 5.0.0 release?

bodograumann · 2020-06-24T09:07:29Z

Thanks for your contribution, @chibat
I think it's a good addition, which increases our reach. (deno is the future?!?)

One small note:
I would have preferred a cleaner commit history to better understand the changes and look at them in chunks.

chibat · 2020-06-27T00:12:47Z

Thanks for your reviews.
I look forward to the future of Deno !
Although the current phase may be for early adopters ..

@bodograumann

I would have preferred a cleaner commit history to better understand the changes and look at them in chunks.

Sorry.. I'll be careful.

chibat added 4 commits June 21, 2020 09:35

add 'deno' to typescript platforms

b5ff53c

Merge branch 'fork_origin_master' into deno

c426c80

Merge remote-tracking branch 'fork_origin/master' into deno

48afe57

add Deno support to typescript generator

393bbcc

TiFu requested changes Jun 21, 2020

View reviewed changes

chibat mentioned this pull request Jun 21, 2020

Deno typescript OpenAPI generators #3174

Closed

add Deno support to typescript generator

334f759

amakhrov reviewed Jun 22, 2020

View reviewed changes

chibat added 2 commits June 22, 2020 21:29

add Deno support to typescript generator

b306168

add Deno support to typescript generator

8a67205

chibat marked this pull request as ready for review June 22, 2020 22:54

add Deno support to typescript generator

9d2d02e

bodograumann reviewed Jun 24, 2020

View reviewed changes

modules/openapi-generator/src/main/resources/typescript/api/api.mustache Outdated Show resolved Hide resolved

chibat added 4 commits June 25, 2020 05:58

add extensionForDeno property for typescript generator

5f23333

add URLParse Deno wrapper for typescript generator

2c2ba9d

Merge remote-tracking branch 'fork_origin/master' into deno

717760d

update deno version in .travis.yml

7e48295

chibat requested a review from TiFu June 27, 2020 00:10

macjohnny merged commit 1f27700 into OpenAPITools:master Jun 27, 2020

chibat deleted the deno branch June 27, 2020 08:35

wing328 added this to the 5.0.0 milestone Jul 3, 2020

arsa-dev mentioned this pull request Oct 22, 2022

Elección del runtime a utilizar arsa-dev/OMP-Logic#12

Closed

danmichaelo mentioned this pull request Jan 3, 2023

[typescript] Add importFileExtension option to support ECMAScript module resolution #14371

Merged

5 tasks

Uh oh!

Conversation

chibat commented Jun 21, 2020 • edited by macjohnny Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR checklist

Uh oh!

TiFu left a comment

Choose a reason for hiding this comment

Uh oh!

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!

TiFu Jun 23, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

amakhrov Jun 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TiFu commented Jun 21, 2020

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

Deno Third Party Modules

Pika

jspm

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

macjohnny commented Jun 24, 2020

Uh oh!

bodograumann commented Jun 24, 2020

Uh oh!

chibat commented Jun 27, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

chibat commented Jun 21, 2020 •

edited by macjohnny

Loading

TiFu Jun 23, 2020 •

edited

Loading

amakhrov Jun 27, 2020 •

edited

Loading