openhab / openhab-js

openHAB JavaScript Library for JavaScript Scripting Automation
https://www.openhab.org/addons/automation/jsscripting/
Eclipse Public License 2.0
38 stars 31 forks source link

Add TypeScript type definitions #137

Closed florian-h05 closed 2 years ago

florian-h05 commented 2 years ago

This PR adds type definitions (.d.ts), which really supercharges code completion in IDEs.

The type definitons are generated from JSDoc. I know that are some problems with @typedef in JSDoc (as mentioned in #46), but I think that I somehow managed that both JSDoc and TypeScript work correctly.

I would prefer to get #136 merged first, because then I can update the type defintions before they are merged into main.

digitaldan commented 2 years ago

This sounds amazing! It something I was working on back in Jan, but never got around to finishing. Just FYI since i went MIA the last few days, I was traveling this weekend, and on Holiday for the week, but i plan on reviewing this and the other PR today.

florian-h05 commented 2 years ago

Just FYI since i went MIA the last few days, I was traveling this weekend, and on Holiday for the week, but i plan on reviewing this and the other PR today.

I hope you enjoyed it! Next week I am also going to be MIA since I am going to go abroad.

Oops, I just closed this PR by accident, but I reopened it.

florian-h05 commented 2 years ago

@digitaldan I updated this PR with the changes from #136.

digitaldan commented 2 years ago

So when i run tsc from the root directory i'm getting the following errors

rules/operation-builder.js:1:1 - error TS9005: Declaration emit for this file requires using private name 'and'. An explicit type annotation may unblock declaration emit.

1 const parseDuration = require('parse-duration');
  ~~~~~

rules/operation-builder.js:1:1 - error TS9005: Declaration emit for this file requires using private name 'toItem'. An explicit type annotation may unblock declaration emit.

1 const parseDuration = require('parse-duration');
  ~~~~~

Found 2 errors in the same file, starting at: rules/operation-builder.js:1

Also should we add typescript as a dev dependency and then add "tsc": "tsc" to the scripts section our package.json ?

florian-h05 commented 2 years ago

Regarding the errors: I forgot those two, I will look if I find a solution.

Also should we add typescript as a dev dependency and then add "tsc": "tsc" to the scripts section our package.json ?

tsc already is a dev dependency, but using the tsc command in a script leads to a tsc error. If you want to use tsc in a npm script you have to install it as normal dependency.

florian-h05 commented 2 years ago

@digitaldan I have fixed the compiler errors, ready for review now.

After this PR has been merged, I will publish a new version (2.0.0 due to some breaking changes) and update your PR in openhab-addons as we do not have much time left to get this into the next openHAB release (feature freeze is late evening this Sunday!!).

The next step after this has been merged could probably be to enable type checking inside the library, but then we need type declarations for Java, @runtime and openHAB core stuff.

florian-h05 commented 2 years ago

@digitaldan

When you install from this PR (with npm install git+https://github.com/florian-h05/openhab-js.git#add-type-definitions), you should get autocompletion for all the exports. The lazy loading (in index.js) caused some heavy trouble, but I was able to solve it by manually creating a main type definition.

digitaldan commented 2 years ago

When you install from this PR (with npm install git+https://github.com/florian-h05/openhab-js.git#add-type-definitions), you should get autocompletion for all the exports.

Thanks for the tip. Auto complete looks good for items, actions, things, etc... but for some reason rules. is not auto completing for me. I have not played with it much more to know if its a problem on my end or not.

tsc already is a dev dependency, but using the tsc command in a script leads to a tsc error. If you want to use tsc in a npm script you have to install it as normal dependency.

So if i add typescript as a dev dependency,, and add tsc to the scripts section, i am able to do a npm run tsc successfully without having to install external dependencies (like typescript) outside of doing a npm install . Thought it might be better for others and also for automating the ts generation from a CI pipeline.

 "scripts": {
        "test": "mocha && npm run lint",
        "docs": "rm -Rf ./docs/* && ./node_modules/.bin/jsdoc --configure docs_config.json && mv ./docs/$npm_package_name/$npm_package_version/* ./docs/ && rm -Rf ./docs/$npm_package_name/$npm_package_version",
        "deploy": "npm test && npm run docs",
        "lint": "npx standardx | snazzy",
        "fix-codestyle": "npx standardx --fix",
        "webpack": "webpack",
        "tsc": "tsc"
    },
 "devDependencies": {
        "acorn": "^8.6.0",
        "dmd-readable": "^1.2.4",
        "docdash": "^1.2.0",
        "jsdoc": "^3.6.3",
        "mocha": "^6.2.2",
        "proxyquire": "^2.1.3",
        "rewiremock": "^3.13.9",
        "snazzy": "^9.0.0",
        "standardx": "^7.0.0",
        "tsc": "^2.0.4",
        "typescript": "^4.7.3",
        "webpack": "^5.58.2",
        "webpack-cli": "^4.9.1"
    }
florian-h05 commented 2 years ago

Auto complete looks good for items, actions, things, etc... but for some reason rules. is not auto completing for me. I have not played with it much more to know if its a problem on my end or not.

I will check if it is working for me later when I'm back home.

So if i add typescript as a dev dependency,, and add tsc to the scripts section, i am able to do a npm run tsc successfully without having to install external dependencies (like typescript) outside of doing a npm install.

Ah okay, I misunderstood your last post, I will try it that approach works for me. Thanks for the tip!

florian-h05 commented 2 years ago

@digitaldan I was able to fix the autocompletion for rules, but RuleBuilder is not in the autocompletion. (The autocompletion for the RuleBuilder is not working on it‘s own, that‘s something to look into later, because we have to hurry and should get this PR merged!!)

I was also able to add a package script to generate the types: npm run types.

Thought it might be better for others and also for automating the ts generation from a CI pipeline.

Regarding an automation for type generation, that‘s definitively something to add later, maybe we can integrate it to my proposed autopublish pipeline (see our team discussions).

digitaldan commented 2 years ago

LGTM, thanks. One thing, which i'm fine if you want to fix in another PR. It looks like tsc has been deprecated in favor of using typescript directly. After reseting my workspace and reinstalling dependencies, tsc was complaining again about not finding typescript, super strange, not sure whats going on there, but I went ahead and uninstalled tsc, and added

 "types": "./node_modules/typescript/bin/tsc"

and now it works. I'm also not sure why just aliasing tsc does not work here since i would assume its suppose to find stuff in the bin dir above, npx does not fins tsc as well 🤷

Feel free to merge this when ready !

florian-h05 commented 2 years ago

It looks like tsc has been deprecated in favor of using typescript directly.

I have just checked and yes, you are right. I removed tsc and reinstalled my workspace and everything works fine with npm run types just being tsc as command.