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

bug: Preact typing doesn't work if using non-legacy TypeScript moduleResolution #9494

Open
2 of 6 tasks
maxpatiiuk opened this issue Jun 3, 2024 · 5 comments
Open
2 of 6 tasks
Labels
0 - new New issues that need assignment. ArcGIS Maps SDK for JavaScript Issues logged by ArcGIS SDK for JavaScript team members. bug Bug reports for broken functionality. Issues should include a reproduction of the bug. calcite-components Issues specific to the @esri/calcite-components package. estimate - 3 A day or two of work, likely requires updates to tests. impact - p3 - not time sensitive User set priority impact status of p3 - not time sensitive p - low Issue is non core or affecting less that 10% of people using the library spike Issues that need quick investigations for time estimations, prioritization, or a quick assessment.

Comments

@maxpatiiuk
Copy link
Member

Check existing issues

Actual Behavior

Calcite provides typings for Preact.
However, these work only if using legacy TypeScript moduleResolution setting "Node"

Expected Behavior

Using modern moduleResolution like "Node16" or "Bundler" should work.
Without this, you can't use the package.json's "exports" field

Reproduction Sample

https://github.com/Esri/calcite-components-examples/tree/main/preact-typescript

Reproduction Steps

  1. Clone this folder: https://github.com/Esri/calcite-components-examples/tree/main/preact-typescript

  2. In tsconfig on this line, replace the legacy module resolution with a modern one:

    -     "moduleResolution": "Node",
    +     "moduleResolution": "Node16", // or "Bundler"
  3. See error anywhere where calcite elements are used in JSX

    Property 'calcite-icon' does not exist on type 'JSX.IntrinsicElements'.ts(2339)

Reproduction Version

2.8.5

Relevant Info

The issue is likely because the preact typings generated by Calcite reference a "preact/src/jsx" file which is not included in Preact's package.json's "exports" field:

Note, because of TypeScript limitation, this might be not possible to solve without opening a PR in Preact to add this to their package.json's exports:

    "./src/jsx": { "types": "./src/jsx.d.ts" },

With the above line, Calcite's typings seem to work again

Regression?

No response

Priority impact

impact - p3 - not time sensitive

Impact

I tried to add Preact typings to arcgis-web-components, but faced an issue.
When I looked at how typings work in Calcite, I noticed that Calcite's Preact typings don't work with newer TypeScript module resolution setting.

Calcite package

  • @esri/calcite-components
  • @esri/calcite-components-angular
  • @esri/calcite-components-react
  • @esri/calcite-design-tokens
  • @esri/eslint-plugin-calcite-components

Esri team

ArcGIS Maps SDK for JavaScript

@maxpatiiuk maxpatiiuk added bug Bug reports for broken functionality. Issues should include a reproduction of the bug. 0 - new New issues that need assignment. needs triage Planning workflow - pending design/dev review. labels Jun 3, 2024
@github-actions github-actions bot added ArcGIS Maps SDK for JavaScript Issues logged by ArcGIS SDK for JavaScript team members. calcite-components Issues specific to the @esri/calcite-components package. impact - p3 - not time sensitive User set priority impact status of p3 - not time sensitive labels Jun 3, 2024
@maxpatiiuk
Copy link
Member Author

maxpatiiuk commented Jun 3, 2024

As per preactjs/preact#1180 (comment), changing the preact.d.ts that Calcite generates like so appears to work both with the legacy and the new moduleResolution strategy:

- import { JSXInternal } from "preact/src/jsx"; 
- import { JSX } from "./components";
+ import { JSX as JSXInternal } from "preact";
+ import { JSX as CalciteJsx } from "./components";

- declare module "preact/src/jsx" {
-   namespace JSXInternal {
+ declare module 'preact' {
+   namespace createElement.JSX extends JSXInternal.IntrinsicElements {

This seems to work because new versions of TypeScript look at the jsx factory function to resolve the JSX namespace

In addition:

  • Rename usages of JSX.Calcite* in the file with CalciteJsx.Calcite*)
  • Duplicate the contents of namespace createElement.JSX extends JSXInternal.IntrinsicElements { ... } and call it namespace h.JSX extends JSXInternal.IntrinsicElements { ... } so that typing works both for people using "jsxFactory": "h" and "jsxFactory": "createElement"`

@paulcpederson Since you worked on the original Calcite types for Preact, could you please verify if the above fix looks good and if so if it can be integrated into the preact.d.ts that Calcite ships?

@geospatialem geospatialem added p - low Issue is non core or affecting less that 10% of people using the library estimate - 3 A day or two of work, likely requires updates to tests. needs milestone Planning workflow - pending milestone assignment, has priority and/or estimate. and removed needs triage Planning workflow - pending design/dev review. labels Jun 10, 2024
@geospatialem geospatialem added the spike Issues that need quick investigations for time estimations, prioritization, or a quick assessment. label Sep 10, 2024
@geospatialem geospatialem removed the needs milestone Planning workflow - pending milestone assignment, has priority and/or estimate. label Sep 10, 2024
@geospatialem
Copy link
Member

Spike to determine if the breaking change release mitigates the above.

@maxpatiiuk
Copy link
Member Author

Note, this issue will be fixed automatically upon migration to Lit, thus you could delay addressing it until then.

@jcfranco
Copy link
Member

Blocked by #10310.

@jcfranco jcfranco added the blocked This issue is blocked by another issue. label Sep 27, 2024
Copy link
Contributor

Issue #10310 has been closed, this issue is ready for re-evaluation.

cc @geospatialem,@DitwanP

@github-actions github-actions bot removed the blocked This issue is blocked by another issue. label Nov 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0 - new New issues that need assignment. ArcGIS Maps SDK for JavaScript Issues logged by ArcGIS SDK for JavaScript team members. bug Bug reports for broken functionality. Issues should include a reproduction of the bug. calcite-components Issues specific to the @esri/calcite-components package. estimate - 3 A day or two of work, likely requires updates to tests. impact - p3 - not time sensitive User set priority impact status of p3 - not time sensitive p - low Issue is non core or affecting less that 10% of people using the library spike Issues that need quick investigations for time estimations, prioritization, or a quick assessment.
Projects
None yet
Development

No branches or pull requests

3 participants