feat: generate immutable types #522

dnalborczyk · 2021-03-15T23:28:39Z

fixes: #501

bin/cli.js

drwpow · 2021-03-15T23:31:07Z

src/transform/headers.ts

  let output = "";

  Object.entries(headerMap).forEach(([k, v]) => {
    if (!v.schema) return;

    if (v.description) output += comment(v.description);
+
+    const readonly = immutableTypes ? "readonly " : "";


I personally am also a fan of readonly string[] over ReadonlyArray<string>. AFAIK this looks easier to implement, and has no downsides. I’m in favor of this approach.

the "downside" - if you want to call it such - would be a minimum requirement of TypeScript v3.4, which I think should be fine considering v4.2 is currently latest.

TS 4 was such an easy update I don’t know of any reason why people would still be on an old version. But given that this is opt-in too, this is more than fine.

drwpow · 2021-03-15T23:33:12Z

Awesome! Love the direction of this PR; I would have done it the same way.

My only suggestion would be to add at least 1 test for this. But the nice part of adding a new feature is that we’re not breaking behavior for anybody. So even if we missed a type, we could add it after-the-fact.

codecov · 2021-03-16T00:11:14Z

Codecov Report

Merging #522 (1b0363e) into main (ccb5f15) will increase coverage by 0.03%.
The diff coverage is 82.71%.

@@            Coverage Diff             @@
##             main     #522      +/-   ##
==========================================
+ Coverage   84.87%   84.91%   +0.03%     
==========================================
  Files           9        9              
  Lines         324      338      +14     
  Branches      105      108       +3     
==========================================
+ Hits          275      287      +12     
- Misses         46       47       +1     
- Partials        3        4       +1

Impacted Files	Coverage Δ
src/transform/index.ts	`57.40% <40.00%> (+0.80%)`	⬆️
src/transform/headers.ts	`27.27% <50.00%> (-2.73%)`	⬇️
src/transform/responses.ts	`87.50% <78.57%> (+0.65%)`	⬆️
src/transform/paths.ts	`92.00% <87.50%> (+0.33%)`	⬆️
src/transform/schema.ts	`96.92% <91.30%> (-1.47%)`	⬇️
src/index.ts	`77.77% <100.00%> (+1.30%)`	⬆️
src/transform/operation.ts	`96.66% <100.00%> (+0.23%)`	⬆️
src/transform/parameters.ts	`97.50% <100.00%> (+0.06%)`	⬆️
src/utils.ts	`90.90% <100.00%> (+0.34%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 57f955d...1b0363e. Read the comment docs.

dnalborczyk · 2021-03-16T00:19:58Z

Awesome! Love the direction of this PR; I would have done it the same way.

Thank you! 😊

My only suggestion would be to add at least 1 test for this.

yeah, I'll add some tests. just wanted to see the current tests passing.

But the nice part of adding a new feature is that we’re not breaking behavior for anybody. So even if we missed a type, we could add it after-the-fact.

I agree.

dnalborczyk · 2021-03-19T02:57:46Z

@drwpow the PR is pretty much good to go. I just want to merge some duplicate interfaces I created for passing around options: { immutableTypes }. gonna try to get to it tomorrow.

I tried the branch against our company schemas and it works great.

drwpow · 2021-03-19T04:55:43Z

Awesome. I’ll take a look tomorrow. It’s looking good to me already.

src/transform/index.ts

drwpow · 2021-03-20T01:29:19Z

tests/operation.test.ts

+            };
+          };
+        };`)
+    );


drwpow · 2021-03-20T01:30:19Z

tests/v3/index.test.ts

+      expect(generated).toBe(expected);
+    });
+
+    it(`reads ${schema} spec (v3) from file (immutable types)`, () => {


Love all the immutable tests!

drwpow

This PR is looking good! I have no changes requested. Ping me when you feel you’re happy with it and I can merge.

I can also either release an RC if you’d like to do more testing, or we can just cut a minor release. Your call. Again, since this is a new flag, I’m not as concerned with just releasing because we won’t be breaking anyone.

dnalborczyk · 2021-03-22T16:54:49Z

This PR is looking good! I have no changes requested. Ping me when you feel you’re happy with it and I can merge.

I can also either release an RC if you’d like to do more testing, or we can just cut a minor release. Your call. Again, since this is a new flag, I’m not as concerned with just releasing because we won’t be breaking anyone.

should be good to go! 👍 minor release should be fine 🤞

dnalborczyk · 2021-03-22T16:55:50Z

@drwpow just wanted to point out a couple things:

1:
removed describe.only, probably a left-over from another PR:
describe.only("transformParametersArray()", () => {

https://github.com/drwpow/openapi-typescript/pull/522/files#diff-af27d23c457e1f97abe9104f8a919b437d93023bdef97136a7cdcb5c36272bb4L2-L3

2:
I applied the prettier config for the cli tests, as it seemed to be more reliable (and also fixable in case of prettier and/or type generator changes:

https://github.com/drwpow/openapi-typescript/pull/522/files#diff-d0f345ad2129eda6bbb5a89e78f0a8f7a4f1e3ee9906893349c96a6686db4dcaR25

execSync(
  `../../bin/cli.js specs/${schema} -o generated/${output} --prettier-config .prettierrc --immutable-types`,
  {
    cwd: __dirname,
  }
);

and therefore also removed the folders from .prettierignore:

https://github.com/drwpow/openapi-typescript/pull/522/files#diff-b640b344ee7f3f03d2a443795a5d0708ef50e2e6e34214109ab2aad13ad6ba98L2-L3

3:

I added an npm prepare script:

https://github.com/drwpow/openapi-typescript/pull/522/files#diff-7ae45ad102eab3b6d7e7896acd08c427a9b25b346470d7bc6507b6481575d519R49

that makes it possible to install this module directly from the repository, as the build automatically runs after the install, which is great for development.

drwpow · 2021-03-24T22:27:20Z

Thanks so much! Agree with all of these changes. Thanks again for contributing

drwpow · 2021-03-24T22:27:41Z

@all-contributors please add @dnalborczyk for code, test

allcontributors · 2021-03-24T22:27:50Z

@drwpow

I've put up a pull request to add @dnalborczyk! 🎉

dnalborczyk · 2021-03-24T23:25:27Z

Thanks so much! Agree with all of these changes. Thanks again for contributing

@drwpow thank you for this great project and thank you for accepting the PR!

laurence-myers · 2021-03-25T07:52:27Z

@drwpow @dnalborczyk Thanks for implementing & reviewing this feature! I really appreciate your time & efforts. 🎉

(I just wanted to join the "thanks" party! 😁)

drwpow reviewed Mar 15, 2021

View reviewed changes

bin/cli.js Outdated Show resolved Hide resolved

drwpow reviewed Mar 15, 2021

View reviewed changes

dnalborczyk force-pushed the immutable-types branch from 6ac95dd to a43ce6b Compare March 15, 2021 23:37

drwpow reviewed Mar 20, 2021

View reviewed changes

src/transform/index.ts Outdated Show resolved Hide resolved

drwpow reviewed Mar 20, 2021

View reviewed changes

tests/operation.test.ts

};

};

};`)

);

Copy link

Contributor

drwpow Mar 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯

drwpow reviewed Mar 20, 2021

View reviewed changes

drwpow approved these changes Mar 20, 2021

View reviewed changes

dnalborczyk added 13 commits March 22, 2021 11:37

feat: generate immutable types

2195b16

fix: remove optional chaining

ace0169

fix: use kebab-case flag

7d1ab06

test: add immutable types tests

4a3b984

test: extend empty definitions with immutable types

d1188ef

test: extend parameters with immutable types

9b813b1

test: extend raw schema with immutable types

ad9b0dd

test: extend operation with immutable types

13c8d7a

test: extend schema with immutable types

6149a09

test: extend paths with immutable types

b2ac8ac

fix: immutable additional properties

52f0aeb

test: add schema v2 immutable types tests

05a29d1

add npm prepare script

02f98e5

dnalborczyk force-pushed the immutable-types branch from 0b31fa6 to 02f98e5 Compare March 22, 2021 15:38

refactor to use function

1b0363e

dnalborczyk marked this pull request as ready for review March 22, 2021 16:55

drwpow merged commit 5c32521 into openapi-ts:main Mar 24, 2021

allcontributors bot mentioned this pull request Mar 24, 2021

docs: add dnalborczyk as a contributor #540

Merged

dnalborczyk deleted the immutable-types branch March 24, 2021 23:15

Uh oh!

feat: generate immutable types #522

feat: generate immutable types #522

Uh oh!

Conversation

dnalborczyk commented Mar 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

drwpow Mar 15, 2021

Choose a reason for hiding this comment

Uh oh!

drwpow Mar 15, 2021

Choose a reason for hiding this comment

Uh oh!

dnalborczyk Mar 17, 2021

Choose a reason for hiding this comment

Uh oh!

drwpow Mar 17, 2021

Choose a reason for hiding this comment

Uh oh!

drwpow commented Mar 15, 2021

Uh oh!

codecov bot commented Mar 16, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

dnalborczyk commented Mar 16, 2021

Uh oh!

dnalborczyk commented Mar 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

drwpow commented Mar 19, 2021

Uh oh!

Uh oh!

drwpow Mar 20, 2021

Choose a reason for hiding this comment

Uh oh!

drwpow Mar 20, 2021

Choose a reason for hiding this comment

Uh oh!

drwpow left a comment

Choose a reason for hiding this comment

Uh oh!

dnalborczyk commented Mar 22, 2021

Uh oh!

dnalborczyk commented Mar 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

drwpow commented Mar 24, 2021

Uh oh!

drwpow commented Mar 24, 2021

Uh oh!

allcontributors bot commented Mar 24, 2021

Uh oh!

dnalborczyk commented Mar 24, 2021

Uh oh!

laurence-myers commented Mar 25, 2021

Uh oh!

Uh oh!

dnalborczyk commented Mar 15, 2021 •

edited

Loading

codecov bot commented Mar 16, 2021 •

edited

Loading

dnalborczyk commented Mar 19, 2021 •

edited

Loading

dnalborczyk commented Mar 22, 2021 •

edited

Loading