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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat/typescript migration #862

Open
wants to merge 86 commits into
base: master
Choose a base branch
from

Conversation

sroucheray
Copy link
Contributor

@sroucheray sroucheray commented Dec 13, 2022

Hi @mrin9,

So here is the branch where I migrated the project to Typescript.

  • Migrate all files from JS to TS
    • Use openapi-types lib wherever possible
    • Add extra typings where it was feasible
    • Cast to typing where some type incompatibility may arise
    • When I had not other way I added some anys where typing were difficult to understand
    • Try NOT to fix code yet, unless typings made it impossible to compile
    • Migrate rapidoc-mini.js (commented out for now to avoid extra work)
  • Update tsconfig and webpack to adapt the build pipeline (commands did not change)
    • build passes and produces artefacts (js and typing files)

The remaining tasks I see

  • Improve / Cleanup custom typings by someone who has more knowledge about the shape of object
    • Cleanup code wherever typings alert
    • Replace remaining any references in the code by proper types
    • Replace remaining 'as ...' type casting added in the code because of lack of knowledge about objects shapes or union types were not properly discriminated in the original code.
    • Check // TODO: Typescript migration... in the code to find mistake in original code now catched by the Typescript compiler (usually it needs refactor or removal)
  • Testing
  • Rebase on master and integrate last modification
  • Simplify and make some portions of code more robust which should be much easier with the help of typings
  • Add unit tests

One note about functions. You'll see that many functions have a first parameter named this, it's not a real parameter but the way typescript make it clear on which type of object the function is called (as many utility functions are called with call). https://www.typescriptlang.org/docs/handbook/2/functions.html#declaring-this-in-a-function

Example

...
export default function jsonSchemaViewerTemplate(this: RapiDocJSONSchemaViewerElement, isMini = false) {
...

@sroucheray
Copy link
Contributor Author

Side note: my last commit is a fix for the response status not properly set when an error occurred. I encountered this error while working on a specific API. I took the opportunity to narrow down the type of the responseStatus to success | error instead of a string. Typescript makes it clear what this property can hold as a value now !

@sroucheray
Copy link
Contributor Author

I merged master in my branch to keep up to date with latest commits

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