[docker] Install tsc so that --emit-tsc works inside the container #1371
Conversation
merceyz
changed the title
docker/Dockerfile
Outdated
| @@ -50,6 +50,12 @@ RUN echo "## Aggressive optimization: Remove debug symbols" \ | |||
| && find ${EMSDK}/upstream/bin -type f -exec strip -s {} + || true \ | |||
| && echo "## Done" | |||
|
|
|||
| RUN echo "## Installing npm dependencies" \ | |||
There was a problem hiding this comment.
The emsdk build script uses does install npm dependencies, just no the devDependencies. It uses npm ci --production --no-optional.
How about emsdk install tsc so that we its more obvious why we are doing this here?
There was a problem hiding this comment.
Hm, in that case perhaps emscripten should move typescript to dependencies instead.
There was a problem hiding this comment.
We decided we didn't want to add the tsc dependency for all users, and instead make it an optional dependencies that certain users can opt into.
We can possibly revisit that decision at a later date.
There was a problem hiding this comment.
Fair enough but why not merge this? Docker says the image is currently 2.76 GB. The difference between just production and all dependencies is ~70 MB.
(An example for something that's currently broken is compiling the tree-sitter WASM library with --emit-tsd.)
There was a problem hiding this comment.
Just logically it makes sense that we are simply adding 'tsc'. Other dependencies should already exist. There is no need to install other dev dependencies here.
So maybe change the to "echo Installing" tsc and then npm install tsc?
There was a problem hiding this comment.
I'm not the original author, so I can't change it but that makes sense to me. (I'm not familiar with how emscripten uses TS, hence my broad suggestion. Just installing tsc is of course ideal.)
But if @merceyz isn't around any more (been inactive for a while), I'm happy to submit a new PR.
There was a problem hiding this comment.
I think we just update this PR.. let me see if I can do that.
sbc100
changed the title
Install tsc during Docker build to support the
--emit-tsdflag.Fixes: #1370