cordova-plugin-googlemaps icon indicating copy to clipboard operation
cordova-plugin-googlemaps copied to clipboard

Implement demo of using JSDoc to support type checking js

Open adamduren opened this issue 6 years ago • 5 comments

@wf9a5m75 I am curious to get your thoughts on adding the type checking to the project without having to convert the entire project to typescript. Would this be something that you are in favor of?

Currently there are types in the ionic-native repo. It seems like it would be beneficial to be able to reference them in this project as well to help keep the types in sync as well as providing type checking to this project which will help reduce chance of errors.

This PR is to demo how it could be achieved. We could export the types from this project using the package.json types | typings field and then the ionic-native plugin could list this as a devDependency. Another option would be to put the types in their own repo in which both this repo and the ionic-native one would depend on but I am leaning towards moving them into this project in order to reduce complexity.

It would be an effort I can help with over time but as we move types over we can list files in the includes field of tsconfig and that will provide type checking moving forward.

adamduren avatar Nov 12 '18 23:11 adamduren

Just for an example here is the output for npm run test catching a typo I made for demonstration purposes.

➜  cordova-plugin-googlemaps git:(discuss-embeding-type-info) ✗ npm run check-types

> [email protected] check-types /Users/adamduren/Workspace/cordova-plugin-googlemaps
> tsc

src/browser/PluginGeocoder.js:160:18 - error TS2551: Property 'positon' does not exist on type 'GeocoderRequest'. Did you mean 'position'?

160     if (!request.positon && request.address) {
                     ~~~~~~~

  typings/index.d.ts:45:3
    45   position?: ILatLng | ILatLng[];
         ~~~~~~~~
    'position' is declared here.

adamduren avatar Nov 12 '18 23:11 adamduren

Type issues would also be caught by TravisCI when merging PRs adding another level of protection.

adamduren avatar Nov 12 '18 23:11 adamduren

Moving to TypeScript enforces me to tons of work. Because of that, I can't do that at this time, maybe in next year.

wf9a5m75 avatar Nov 13 '18 00:11 wf9a5m75

Thanks for taking a look, I'll keep the PR open and update the branch as I have more time. One good thing about the approach is that it's a change that can happen incrementally vs all or nothing.

adamduren avatar Nov 14 '18 17:11 adamduren

It's time to migrate to TypeScript.

wf9a5m75 avatar Aug 24 '20 23:08 wf9a5m75