TypeScript as a dev dependency adds 33MB to node_modules. I want to support TypeScript, but I don't want to add this weight to every Hyperapp install. Is there any way we can still distribute typings.d.ts without having TypeScript as a dev dependency on this repo?
What other options do we have?
We could publish the typings to DefinitelyTyped and tell users that want TypeScript support to install them via npm install @typings/hyperapp.
On the other hand if we are in the TypeScript zone already why don't we write the library itself in TypeScript. Andre Staltz argues that "All JS Libraries should be authored in TypeScript
" (or another typed JS layer) and I kinda agree with him.
I'm a bit confused - I thought that TypeScript support comes from adding "typings": "hyperapp.d.ts" to our package.json and including that file when we publish.
I also thought the only reason we install TypeScript was to run the test that verifies our typings match the current library code. We could move this verification to another project since hopefully the API changes should be minimal in a post 1.0.0 world 馃檹
The best place for typings are DefinitelyTyped and flow-typed if source code is plain javascript because they have tests for typings, compatibility checks with typed language version, etc.
I vote for removing typings from core :)
So if we do decide to publish on DefinitelyTyped instead, that might raise the question again of where our API docs are. I think we're going to need to write some if we want to be taken seriously and increase adoption. With the stable 1.0.0 API, this should be worth the effort.
@okwolf Where are they now?
@JorgeBucaran I thought our current answer is that hyperapp.d.ts is our only API documentation.
@okwolf Gotcha. Yeah, you are right.
Is having TypeScript as a dev dependency a big deal? The only people this matters to are the people who clone the project and install its dependencies.
Leaving the type definitions in this repository will lower the overhead of the process of updating the dependencies. It means cloning DefinitelyTyped (which is well over 33MB) and submitting a pull request there.
By leaving the type definitions here, you'll be able to update and release them quite a lot faster.
It also means that the type definitions included in a typical install of hyperapp should match the version that they've installed.
@Seikho We would lose some of that if we removed the typings from this repo. As a compromise, we could ship the typings, but not require TypeScript as a dependency like @okwolf said.
And how about maintaining our own typing repo ?
People will install @hyperapp/typed and we will be able to provide types for Typescript, Flow, Reason, whatever without noise in the main repository
That's another good idea.
Please don't remove the typings from the repo. Just leave it there and remove the typescript dependency as people who use typescript have it probably already installed anyway.
And sorry, but 33MB... it's next to nothing compared to when you do an npm install in a normal project ;)
@alber70g I am without. I hate using DefinitelyTyped and it is really a please to only have to depend on the lib you want to use (Hyperapp) instead of having to depend on one more package to have the types.
Do not forget we will have to do the same with the router and html.
"dependencies": {
"@hyperapp/HTML": "^0.5.1",
"@hyperapp/router": "^0.4.1",
+ "@types/hyperapp": "^1.0.0",
+ "@types/hyperapp-html": "^0.5.1",
+ "@types/hyperapp-router": "^0.4.1",
"hyperapp": "^1.0.0"
}
We could also make TypeScript and _optional_ dependency. If you don't want to install it you run npm install --no-optional
I'm late to the party and don't have much new to add to the thread, but having used TS for a while here are my 2cs
My preference would be be to get to the point where the library is written in TS and published with the typings in npm package for anyone using TS. However I accept this is extra work for the library maintainers and the types can get pretty complex, especially when generics are used (see cyclejs)
@timjacobi Not too bad :)
@SteveALee Please see https://github.com/hyperapp/hyperapp/issues/494
@JorgeBucaran yeah, the 2 are tightly coupled :)
@JorgeBucaran Hmm, I got a bit carried away and answered more than this issue. Sorry
Given the typings are to be maintained separately from the javascript (#494) they are so called ambient types like any others in definitetyped etc. As a TS user of the hyperapp lib I'd like them to be easily available and while that means having them in the main npm package I'd still accept them being available via adding @types/hyperapp as another dependency, especially as I believe @code will still find them anyway (if names are consistent).
@timjacobi's or @okwolf suggestions are pretty good :)
Thank you, everyone, for your feedback. I think I'll keep things the way they are for now, or use @okwolf's idea, either way, we'll keep the typings on this repo.
I saw this a few days ago, thought I'd add my two cents even though it's been closed.
Since you're not distributing the library in Typescript form, you could easily remove Typescript from devDependencies all together since it's technically not a dependency.
Really, the only part of the lib that requires Typescript is the tests. That can be resolved by replacing the test command with (npm list typescript || npm i typescript) && jest --coverage --no-cache && tsc -p test/ts to make sure it's installed if tests are being ran.
Hey @ClickSimply! Wow, that's really ingenious:
(npm list typescript || npm i typescript) && jest --coverage --no-cache && tsc -p test/ts
Sneaky! 馃悕
@ClickSimply or you could just simply do jest --coverage --no-cache && npx typescript -p test/ts
@okwolf But then we need to install npx! 馃槒
@JorgeBucaran if only npx came with node 8+. Oh wait...
@okwolf It does?
@JorgeBucaran technically npx comes bundled with npm >= 5.2.0, which comes bundled with Node.js >= 8.2.0 馃檮
My suggestion was intentional, if you use npx it will take the time to install typescript every time you run the test. The provided command will only install it the first time if it isn't already there.
Either case should work out fine and allow Typescript to be removed from the package file.
Thanks, for clarifying @ClickSimply, I thought npx would do that too, but as it turns out it doesn't. I wonder why.
It's a feature; part of the point of npx is to keep your environment clean so it removes all the dependencies required to run the command after it's finished.
Now we reach 1.0.0 the API is not going to change a lot. Do we really need to test typing ?
Answering no could totally solve the issue.
How common is it to test types? Just curious.
And btw, the types are probably going to change again soon (/cc @Alber70g), so we may want to put this on hold until then.
Well that鈥檚 a good point. The types only change if the api changes. But, if they are published then they need to be tested first.
Given they are effectively an extra part that is published with the main code why not add the tsc install and types test to a separate prepublishOnly, script? That won鈥檛 then run in automated tests like CI so I鈥檇 actually put running of types test in its own script that can be run separately and call that from prepublishOnly. The reason not to use prepublish is that get calls from install too.
That way, the types get tested before publishing but the over head of installing typescript (wors if using npx) is not part of running the main tests.
npx only does an on-demand install if it's not already available. It gives the user control by first checking if the package is installed in node_modules or globally first.
Maybe I'm using it wrong then? It installs Typescript and runs it from the temporary location every time I run it. Might be a Windows thing.
Edit: Hmm, seems like there's an open issue with npx regarding this. Might just be a temporary bug.
Documented behavior is exactly as you described okwolf. I've just never seen it. ;)
@JorgeBucaran
TypeScript as a dev dependency adds 33MB to node_modules. I want to support TypeScript, but I don't want to add this weight to every Hyperapp install.
Who are you worried about downloading the 33MB?
The way I see it, there are only two groups of people who could be affected: contributors or users.
git clone https://github.com/hyperapp/hyperapp && npm installnpm install --save hyperappBecause the users will not get typescript because its a devDependency. Only the contributors will get it which makes sense because they need to run tests. So any way you slice it, at some point a contributor will need to install typescript via npm install or npx or copy files from a tar ball....it all ends up on their computer to run the tests.
I've gotta say my thoughts and preference are in line with @JorgeBucaran. The offered solutions do work but add complexity to save devs a few meg storage.
@styfle
Because the users will not get typescript because its a devDependency.
This isn't correct. npm will install devDependencies unless the --production flag is passed to the npm install command OR if the NODE_ENV is set to production.
@ClickSimply I think you missed a key point in my post above....
When I say "users" I mean the users of hyperapp...the people who install hyperapp. These people will NOT get TypeScript, and I can prove it.
Execute the following:
mkdir my-app
cd my-app
npm init -y
npm install --save hyperapp
# + [email protected]
# added 1 package in 1.338s
ls -l node_modules
# total 4
# drwxr-xr-x 1 styfle 197609 0 Jan 13 17:09 hyperapp/
du -sh .
# 82K .
And there you have it, the only dependency install is hyperapp and its only 82K, not 33M.
This is because hyperapp has no dependencies, only devDependencies.
However, the "contributors", the people who develop hyperapp and contribute Pull Requests will get the devDependencies when they clone hyperapp and run npm install. Which is basically what your post describes and your link to npm cli.
Most helpful comment
@JorgeBucaran Hmm, I got a bit carried away and answered more than this issue. Sorry
Given the typings are to be maintained separately from the javascript (#494) they are so called ambient types like any others in definitetyped etc. As a TS user of the hyperapp lib I'd like them to be easily available and while that means having them in the main npm package I'd still accept them being available via adding
@types/hyperappas another dependency, especially as I believe @code will still find them anyway (if names are consistent).