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

Typescript appears in the generated package.json #524

Closed
odisseus opened this issue Mar 20, 2023 · 4 comments · Fixed by #525
Closed

Typescript appears in the generated package.json #524

odisseus opened this issue Mar 20, 2023 · 4 comments · Fixed by #525

Comments

@odisseus
Copy link
Contributor

odisseus commented Mar 20, 2023

I have a project which uses the ScalablyTyped plugin together with sbt-scalajs-bundler. I have noticed that "typescript": "4.3" appears in the dependencies section of the generated package.json, even though none of my npmDependencies depend on Typescript (however, I have some Typescript typings in my npmDevDependencies).

I think this behaviour is caused by these lines:

val deps = (Compile / npmDependencies).value
/* Make sure we always include typescript for the stdlib if it wasnt already added */
val withTypescript: Seq[(String, String)] =
if (deps.exists { case (lib, _) => lib == "typescript" }) deps
else deps :+ ("typescript" -> stTypescriptVersion.value)

From my understanding (admittedly very limited) of the Javascript ecosystem, this is almost certainly wrong. Since the output of the Scala.js compiler is pure Javascript, it doesn't depend on Typescript unless it has other dependencies that need it.

If the ScalablyTyped plugin requires Typescript to function, I think it should be appended to npmDevDependencies instead.

@oyvindberg
Copy link
Collaborator

oyvindberg commented Mar 20, 2023

Hey there. So as you might guess, typescript is not added to the production dependency list for no reason :)

The essence is that ST ignores dev dependencies by default (though that is configurable). This because the dependency graph gets unwieldy real quick. Changing this would have performance implications (ST is already slow, and adding dozens to hundreds of libraries more would be even worse). Also when you disregard dependency versions and only use the newest version of each dependency (like ST does) the graph is also frequently circular, which causes issues since those circles must be broken in somewhat arbitrary ways.

I'm sure there are improvements we can do here, but first I'm wondering if this is actually a problem?

@odisseus
Copy link
Contributor Author

One problem that I've personally encountered is the size of a final artifact being significantly increased. But I think that adding a new dependency that's actually not needed is a bad idea in general, regardless whether it causes any immediate problems.

@oyvindberg
Copy link
Collaborator

Maybe chase down why all of typescript ends up your js bundle? There must be a reference to it somehow, it's not like all dependencies are inlined into the bundle otherwise. The difference between the dependency sets is not that big actually, it mostly impacts downstream libraries.

@odisseus
Copy link
Contributor Author

I'm not sure I understood your comment correctly, but I have built a custom version of this plugin (with the changes from PR #525 applied), and there was no more Typescript in the dependencies section of my package.json. Clearly it was coming from the plugin and nothing else.

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 a pull request may close this issue.

2 participants