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

Max parameters exceeded (and other strange things) #588

Open
steinybot opened this issue Dec 11, 2023 · 2 comments
Open

Max parameters exceeded (and other strange things) #588

steinybot opened this issue Dec 11, 2023 · 2 comments

Comments

@steinybot
Copy link
Contributor

Reproduction: https://github.com/steinybot/bug-reports/tree/scalablytyped/max-parameters-exceeded

NOTE: This is using the changes in #586

Trying to import and compile this:

ThisBuild / scalaVersion := "2.13.11"

enablePlugins(ScalaJSPlugin, ScalablyTypedConverterGenSourcePlugin)

organization := "com.example"
stOutputPackage := "web.typings"
stFlavour := Flavour.Slinky
stIgnore ++= List(
  // Ignore this otherwise it fails with: anon is already defined as package anon
  "@ant-design/cssinjs",
  // For whatever reason when we ignore this we get three additional properties:
  // linkDecoration, linkFocusDecoration, and linkHoverDecoration.
  "csstype"
)
Compile / npmDependencies ++= Seq(
  "antd" -> "5.12.1"
)
Compile / npmDevDependencies ++= Seq(
  // Ignore this otherwise it fails with: anon is already defined as package anon
  // For some reason...
  "@types/react" -> "18.2.13"
)
stSourceGenMode := SourceGenMode.Manual(
  (Compile / sourceDirectory).value / "scala"
)

Fails with:

[error] /Users/jason/src/bug-reports/src/main/scala/web.typings/antd/components/Bordered.scala:13:7: Platform restriction: a parameter list's length cannot exceed 254.
[error]   def apply(
[error]       ^
[error] /Users/jason/src/bug-reports/src/main/scala/web.typings/antd/components/Ellipsis.scala:13:7: Platform restriction: a parameter list's length cannot exceed 254.
[error]   def apply(
[error]       ^
[error] /Users/jason/src/bug-reports/src/main/scala/web.typings/antd/components/Expand.scala:13:7: Platform restriction: a parameter list's length cannot exceed 254.
[error]   def apply(
[error]       ^
[error] /Users/jason/src/bug-reports/src/main/scala/web.typings/antd/components/Filter.scala:13:7: Platform restriction: a parameter list's length cannot exceed 254.
[error]   def apply(
[error]       ^
[error] /Users/jason/src/bug-reports/src/main/scala/web.typings/antd/components/Fixed.scala:13:7: Platform restriction: a parameter list's length cannot exceed 254.
[error]   def apply(
[error]       ^
[error] /Users/jason/src/bug-reports/src/main/scala/web.typings/antd/components/Radius.scala:13:7: Platform restriction: a parameter list's length cannot exceed 254.
[error]   def apply(
[error]       ^
[error] /Users/jason/src/bug-reports/src/main/scala/web.typings/antd/components/Selection.scala:13:7: Platform restriction: a parameter list's length cannot exceed 254.
[error]   def apply(
[error]       ^
[error] /Users/jason/src/bug-reports/src/main/scala/web.typings/antd/components/Size.scala:13:7: Platform restriction: a parameter list's length cannot exceed 254.
[error]   def apply(
[error]       ^
[error] /Users/jason/src/bug-reports/src/main/scala/web.typings/antd/components/Sorter.scala:13:7: Platform restriction: a parameter list's length cannot exceed 254.
[error]   def apply(
[error]       ^
[error] /Users/jason/src/bug-reports/src/main/scala/web.typings/antd/components/Sticky.scala:13:7: Platform restriction: a parameter list's length cannot exceed 254.
[error]   def apply(
[error]       ^
[error] /Users/jason/src/bug-reports/src/main/scala/web.typings/antd/components/Summary.scala:13:7: Platform restriction: a parameter list's length cannot exceed 254.
[error]   def apply(
[error]       ^
[error] /Users/jason/src/bug-reports/src/main/scala/web.typings/antd/components/Virtual.scala:13:7: Platform restriction: a parameter list's length cannot exceed 254.
[error]   def apply(
[error]       ^
[error] 12 errors found

There are also some other weird things going on here which could be related to #586, I'm not too sure.

If we do not ignore "@ant-design/cssinjs" then there is a duplicate anon package.

What is really strange is that excluding the dev dependency on "@types/react" -> "18.2.13" also leads to this duplicate anon package. Why would having fewer dependencies have more packages?

Also if we do not ignore "csstype" then we get three fewer properties and we do not exceed the parameter limit. This is also very odd. Why does ignoring it lead to more properties?

Sometimes to pickup the changes we have to bust the cache manually:

rm -rf ./target/streams/_global/stImportSources/_global/streams/input.json && sbt 'stImportSources;compile'
@oyvindberg
Copy link
Collaborator

Hey there

So yeah, a bunch of things here.

too many parameters

The error you're seeing is that the react component identification code mistakenly identifies some code as react components, for instance the default member here. I don't really think I can fix it, since the name and signature is sufficiently similar to a react component that if I do fix it we'll lose other components in other libraries. Would be interesting to see if the typescript compiler accepts it as a react component actually.

The type which is taken to be the type of props has more than 254 required members, so that's where Scala.js starts to complain when they are all put into the apply method which initializes the react component builder.

I think I'll hack around it and say if a component has more than X required members, where X is above say 50, we drop it. nobody is gonna supply more than 50 required props anyway.

anon

anon is a package or object we create so there is a place to put types which didn't have a name in typescript. These could theoretically be output as structural types now, but it's not so convenient to use. So the existence of the package or object depends on whether there were any such types in the typescript code.

Whether it's a package or an object depends on the number of classes inside it. It may sound ridiculous for this to flip-flop like this, but it's a compromise between git (which likes fewer files) and intellij (which does not like scala files with thousands of lines of code)

There are some details surrounding this, where a given anonymous type may be imported from a dependency, and the scala anon class ends up in that library instead and the downstream library will refer to it by that name. I suppose this can explain what you're seeing with anon package appearing and disappearing

if you're seeing anon is already defined as package anon again, could you see if there is both an object and a package in the generated code? I think it's more likely not invalidated by zinc, so an sbt clean should fix it. Otherwise this is a bug.

A final thing here is a suggestion to only use stIgnore when needed (failing import/compile). it's possible that names are resolved to the wrong thing, if there are similarly named things in the global namespace of another library.

ST not picking up changes.

Might it be that ST is not picking up changes in Compile / npmDevDependencies? because that is only taken into consideration if you set stIncludeDev := true. It's not on by default as it may affect the generated result negatively. The reason is that npm depenencies may have circular dependencies, while ST needs to be strict and create a tree of libraries. As such, considering dev dependencies "earlier" may cause a suboptimal tree shape to be chose, and a bunch of types set to Any when dependency links are broken.

if you see any other changes not being picked up by ST I'd love some details, as that would be a very irritating bug. you should not have to delete those files ever.

@oyvindberg
Copy link
Collaborator

FWIW I simplified the build a bit:

organization := "com.example"
stOutputPackage := "web.typings"
stFlavour := Flavour.Slinky
Compile / npmDependencies ++= Seq(
  "antd" -> "5.12.1",
  "@types/react" -> "18.2.13"
)
stSourceGenMode := SourceGenMode.Manual((Compile / sourceDirectory).value / "scala")

ran stImportSources with a snapshot of ST master, compiled, and it worked. if you're building antd wrappers you should add this as well

stMinimize := Selection.AllExcept("antd")

as well to dramatically reduce the amount of code

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

No branches or pull requests

2 participants