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

Failed to import - react-router-dom #338

Closed
KyleJune opened this issue May 30, 2022 · 7 comments
Closed

Failed to import - react-router-dom #338

KyleJune opened this issue May 30, 2022 · 7 comments
Labels
deno Not working in Deno

Comments

@KyleJune
Copy link

Failing module

/** @jsx React.createElement */
/** @jsxFrag React.Fragment */

import React from "https://esm.sh/react@18.1.0?target=deno&pin=v83";
import {
  NavLink,
} from "https://esm.sh/react-router-dom@6.3.0?target=deno&pin=v83&deps=react@18.1.0,react-dom@18.1.0";

export const Example = () => (
  <div>
    <NavLink
      key="example"
      to="/"
      end
      className={({ isActive }) => ([
        isActive
          ? "bg-gray-900 text-white"
          : "text-gray-300 hover:bg-gray-700 hover:text-white",
        "block px-3 py-2 rounded-md text-base font-medium",
      ].join(" "))}
    >
      Example
    </NavLink>
  </div>
)

Error message

vs code shows "Binding element 'isActive' implicitly has an 'any' type. deno-ts(7031)". I wasn't able to make a minimal reproduction path where it has an error on run but vs code will show that error on the isActive argument for the className.

Below is an example error during runtime from my actual code.

TS7031 [ERROR]: Binding element 'active' implicitly has an 'any' type.
      {({ active }) => (
          ~~~~~~
    at file:///home/kyle/Projects/deno/udibo/client/components/navigation/dropdowns/items.tsx:19:11

Additional info

  • esm.sh version: v83
  • Deno version: 1.22.1

I had no issues previously when using v78. Here are the types for the NavLink component in react-router-dom's types file. It appears to be unchanged between v78 and v83 of esm.sh so I'm not sure why it suddenly doesn't recognize the isActive className signature.

/**
 * The public API for rendering a history-aware <a>.
 */
export declare const Link: React.ForwardRefExoticComponent<LinkProps & React.RefAttributes<HTMLAnchorElement>>;
export interface NavLinkProps extends Omit<LinkProps, "className" | "style" | "children"> {
    children?: React.ReactNode | ((props: {
        isActive: boolean;
    }) => React.ReactNode);
    caseSensitive?: boolean;
    className?: string | ((props: {
        isActive: boolean;
    }) => string | undefined);
    end?: boolean;
    style?: React.CSSProperties | ((props: {
        isActive: boolean;
    }) => React.CSSProperties);
}
/**
 * A <Link> wrapper that knows if it's "active" or not.
 */
export declare const NavLink: React.ForwardRefExoticComponent<NavLinkProps & React.RefAttributes<HTMLAnchorElement>>;

When I had all my esm modules pinned to version v78, I was not getting errors like this. When I updated to v83 I got errors in multiple errors like this all related to packages that depend on react. I believe the issue is actually related to the react types. I get a similar issue with ErrorBoundary which comes from react-error-boundary.

error: TS2607 [ERROR]: JSX element class does not support attributes because it does not have a 'props' property.
      <ErrorBoundary
      ^
    at file:///home/kyle/Projects/deno/udibo/client/error.tsx:115:7

TS2786 [ERROR]: 'ErrorBoundary' cannot be used as a JSX component.
  Its instance type 'ErrorBoundary' is not a valid JSX element.
    Type 'ErrorBoundary' is missing the following properties from type 'ElementClass': context, setState, forceUpdate, props, refs
      <ErrorBoundary
       ~~~~~~~~~~~~~
    at file:///home/kyle/Projects/deno/udibo/client/error.tsx:115:8
@KyleJune KyleJune added the deno Not working in Deno label May 30, 2022
@KyleJune
Copy link
Author

It looks like v78 might have been broken too, I'm getting the same errors in my CI for v78 which I don't get locally. Probably due to deno using a cached version of v78 on my local machine. I'm guessing the issue is related to the esm change to using react@^18 and react-dom@^18. I added a comment on my issue for react-dom linked below. To fix that one, I had to update types to pin a specific version of react-dom instead of using ^18. It would be difficult to do the same for all these depenencies since doing that with the deps flag doesn't work. I believe I would have to vendor those dependencies and modify the vendored versions to use correct react/react-dom versions.

#326 (comment)

It's unfortunate that even with version pinning I end up having my dependencies break due to changes to esm.sh. I'd use deno vendor to vendor my dependencies but I don't believe that works on deno deploy...

https://deno.land/manual/tools/vendor#vendoring-dependencies

@KyleJune
Copy link
Author

It looks like deno deploy does support vendoring and import maps now. I tried using vendoring for my existing cached modules but the vendor command won't work because of invalid imports in the types files I have cached. Odd that it's not an issue for running or linting my project but is for vendoring it. The issue is at the top of the file it does an import of "react" which is not a valid path.

https://esm.sh/v78/react-router-dom@6.3.0/X-ZC9yZWFjdC1kb21AMTguMS4wLHJlYWN0QDE4LjEuMA/index.d.ts

import * as React from "react";

Then deno vendor logs an error saying the following.

error: Relative import path "react" not prefixed with / or ./ or ../

I had a similar issue for another module too that doesn't use react. Here is a minimal reproduction path for the vendor issue. It happens with both v78 and v83 of esm.sh. It's the second import that has a types file that imports "twind".

export * as twind from "https://esm.sh/twind@1.0.0-next.38?target=deno&pin=v78";
export { default as presetTailwind } from "https://esm.sh/@twind/preset-tailwind@1.0.0-next.38?target=deno&pin=v78";
deno vendor --force example.ts
error: Relative import path "twind" not prefixed with / or ./ or ../

@KyleJune
Copy link
Author

I was able to workaround the issue with deno vendor from my last comment using the following import map.

{
  "imports": {
    "react": "https://esm.sh/react@18.1.0?target=deno&pin=v78",
    "twind": "https://esm.sh/twind@1.0.0-next.38?target=deno&pin=v78"
  }
}

@KyleJune
Copy link
Author

Vendoring didn't fix my issues. Revealed new issue related to type checking. Apparently even though I pin all my dependencies to v78, it ends up using v83...

image

I was using --no-check=remote before which prevented me from seeing the type errors related to 2 different versions of react and other modules being used.

TS2717 [ERROR]: Subsequent property declarations must have the same type.  Property 'view' must be of type 'SVGProps<SVGViewElement>', but here has type 'SVGProps<SVGViewElement>'.
            view: React.SVGProps<SVGViewElement>;
            ~~~~
    at file:///home/kyle/Projects/deno/udibo/vendor/cdn.esm.sh/v78/@types/react@18.0.6/index.d.ts:3309:13

    'view' was also declared here.
                view: React.SVGProps<SVGViewElement>;
                ~~~~
        at file:///home/kyle/Projects/deno/udibo/vendor/esm.sh/v83/@types/react@18.0.9/X-ZC9yZWFjdEAxOC4xLjA/index.d.ts:3315:13

Found 179 errors.

@KyleJune
Copy link
Author

It's the import * as React from "react"; in the type files causing issue with v83. I'm able to upgrade to v83 if I use the following import map.

{
  "imports": {
    "react": "https://esm.sh/react@18.1.0?target=deno&pin=v83"
  }
}

There are still the above issues when trying to pin to v78.

@KyleJune
Copy link
Author

I've got another issue after doing that fix. When react-router-dom imports react, it uses the url "/v83/react@18.1.0/X-ZC9yZWFjdC1kb21AMTguMS4w/deno/react.js" which is different than the version I get when I import "https://esm.sh/react@18.1.0?target=deno&pin=v83". The direct import of react uses the url "https://esm.sh/v83/react@18.1.0/deno/react.js"

I was able to fix the issue with the following import map so that I'm only using a single version of react.

{
  "imports": {
    "react": "https://esm.sh/react@18.1.0?target=deno&pin=v83",
    "https://esm.sh/v83/react@18.1.0/X-ZC9yZWFjdC1kb21AMTguMS4w/deno/react.js": "https://esm.sh/react@18.1.0?target=deno&pin=v83"
  }
}

The issue presented itself with the following error.

TypeError: Cannot read properties of null (reading 'useContext')
    at n.useContext (https://esm.sh/v83/react@18.1.0/X-ZC9yZWFjdC1kb21AMTguMS4w/deno/react.js:2:6298)
    at useInRouterContext (https://esm.sh/v83/react-router@6.3.0/X-ZC9yZWFjdC1kb21AMTguMS4wLHJlYWN0QDE4LjEuMA/deno/react-router.js:2:5113)
    at Router (https://esm.sh/v83/react-router@6.3.0/X-ZC9yZWFjdC1kb21AMTguMS4wLHJlYWN0QDE4LjEuMA/deno/react-router.js:2:7603)
    at Jc (https://esm.sh/v83/react-dom@18.1.0/X-ZC9yZWFjdEAxOC4xLjA/deno/server.js:12:11257)

It doesn't work with deno vendoring though. I get some ts errors. I think I wasn't getting them when the module was remote because of no check but if I'm vendoring they are local and get checked.

TS2694 [ERROR]: Namespace '"file:///home/kyle/Projects/deno/udibo/vendor/esm.sh/react@18.1.0.js"' has no exported member 'ReactNode'.
    children?: React.ReactNode;
                     ~~~~~~~~~
    at file:///home/kyle/Projects/deno/udibo/vendor/esm.sh/v83/react-router-dom@6.3.0/X-ZC9yZWFjdC1kb21AMTguMS4wLHJlYWN0QDE4LjEuMA/server.d.ts:5:22

TS2503 [ERROR]: Cannot find namespace 'JSX'.
export declare function StaticRouter({ basename, children, location: locationProp, }: StaticRouterProps): JSX.Element;
                                                                                                          ~~~
    at file:///home/kyle/Projects/deno/udibo/vendor/esm.sh/v83/react-router-dom@6.3.0/X-ZC9yZWFjdC1kb21AMTguMS4wLHJlYWN0QDE4LjEuMA/server.d.ts:12:107

I guess I'm going to go without vendoring for now and hope it doesn't break again.

@ije
Copy link
Member

ije commented May 31, 2022

thanks for the details, seems the dts transformer work incorrectly. I will look into!

@ije ije closed this as completed in 004d4d8 May 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deno Not working in Deno
Projects
None yet
Development

No branches or pull requests

2 participants