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

Fix imports/exports from local sibling modules #639

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

steinybot
Copy link
Contributor

@steinybot steinybot commented Jul 29, 2024

Fixes #638

Consider the following structure:

- a/index.d.ts
- b/index.d.ts
- package.json

Where package.json has:

  "types": "b/index.d.ts",

and b/index.d.ts has:

export * from '../a';

Currently this would not generate any of the types from b.

It seems it was intentional to only consider files from types and typings, or the module but this causes lookups from sibling modules to fail.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like how it does this. Since they are re-exported from the main module they should be there instead / as well. My previous attempt added the files to shortenedFiles which did the right thing in that regards but it messed up other stuff. That is probably a separate issue and would change a lot of the way things are generated whereas this change seems relatively isolated.

@steinybot
Copy link
Contributor Author

There might be something not right here. It's getting confused between es and lib. For example:

[error]  found   : web.typings.antd.esButtonButtonMod.ButtonProps with web.typings.react.mod.RefAttributes[org.scalajs.dom.HTMLElement]
[error]  required: web.typings.antd.libButtonButtonMod.ButtonProps with web.typings.react.mod.RefAttributes[org.scalajs.dom.HTMLElement]

@steinybot
Copy link
Contributor Author

Ok I think that's fixed now. The problem was that it was adding a ModuleAliases from es to lib which was overwriting and/or higher priority than the normal module scope.

@steinybot
Copy link
Contributor Author

Interestingly this fixes another existing bug which is the opposite to the problem I had above.

@ant-design/icons/es/components/AntdIcon.d.ts has this import:

import type { IconDefinition } from '@ant-design/icons-svg/lib/types';

Which used to incorrectly resolve to antDesignIconsSvg.esTypesMod.IconDefinition but now it correctly resolves to antDesignIconsSvg.libTypesMod.IconDefinition.

@steinybot
Copy link
Contributor Author

Ughhh so the main entry wasn't being handled properly either (see zootools test).

I have no idea why fixing that triggered a whole lot of changes in the react-integration-tests. Seems like it is all new stuff which is good in a way but the Readonly etc stuff seems odd. Is that supposed to be special cased somewhere?

],
"main": "lib/index.js",
"module": "es/index.js",
"unpkg": "dist/antd.min.js",
"typings": "lib/index.d.ts",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The real package uses es typings.

scalaVersion := "3.3.1"
enablePlugins(ScalaJSPlugin)
libraryDependencies ++= Seq(
"com.github.japgolly.scalajs-react" %%% "core" % "2.1.1",
"com.olvind" %%% "scalablytyped-runtime" % "2.4.2",
"org.scalablytyped" %%% "react" % "16.9.2-eebc56",
"org.scalablytyped" %%% "react" % "16.9.2-d4e96d",
"org.scalablytyped" %%% "react-bootstrap" % "0.32-edeae4",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is this coming from?

scalaVersion := "3.3.1"
enablePlugins(ScalaJSPlugin)
libraryDependencies ++= Seq(
"com.github.japgolly.scalajs-react" %%% "core" % "2.1.1",
"com.olvind" %%% "scalablytyped-runtime" % "2.4.2",
"org.scalablytyped" %%% "react" % "16.9.2-eebc56",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where did this go?

japgolly.scalajs.react.facade.React.Component[ButtonGroupProps & js.Object, js.Object]
] {
/* import warning: RemoveDifficultInheritance.summarizeChanges
- Dropped / * import warning: transforms.QualifyReferences#resolveTypeRef many Couldn't qualify React.HTMLProps<ButtonGroup> * / any */ trait ButtonGroupProps extends StObject {
Copy link
Contributor Author

@steinybot steinybot Aug 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why dropped all of a sudden?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does the error look whack? Seems like a double error or something.

@steinybot
Copy link
Contributor Author

The PhaseRunner is now running Phase1ReadTypescript twice on react for some reason and so isCircular ends up true and it inserts Ignore into the PhaseCache so it ends up being missing from the moduleScopes. I have no idea how this is supposed to work...

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 this pull request may close these issues.

Importing @stripe/stripe-js generates no types
1 participant