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 setup not working as expected #320

Closed
cyrilchapon opened this issue Aug 25, 2023 · 7 comments
Closed

Typescript setup not working as expected #320

cyrilchapon opened this issue Aug 25, 2023 · 7 comments

Comments

@cyrilchapon
Copy link

cyrilchapon commented Aug 25, 2023

Issue summary

After "activating" Typescript in a fresh install, and setting up a typecheck script as recommended in Remix official docs; I'm getting unexpected errors.

$ tsc
app/entry.server.tsx:23:30 - error TS2345: Argument of type 'NodeRequest' is not assignable to parameter of type 'Request'.

23   addDocumentResponseHeaders(request, responseHeaders);
                                ~~~~~~~

app/routes/_index/route.tsx:6:25 - error TS2307: Cannot find module './style.css' or its corresponding type declarations.

6 import indexStyles from "./style.css";
                          ~~~~~~~~~~~~~

app/routes/app.tsx:3:27 - error TS2307: Cannot find module '@shopify/polaris/build/esm/styles.css' or its corresponding type declarations.

3 import polarisStyles from "@shopify/polaris/build/esm/styles.css";
                            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

app/routes/auth.login/route.tsx:14:27 - error TS2307: Cannot find module '@shopify/polaris/build/esm/styles.css' or its corresponding type declarations.

14 import polarisStyles from "@shopify/polaris/build/esm/styles.css";
                             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

node_modules/@remix-run/node/dist/fetch.d.ts:3:106 - error TS1479: The current file is a CommonJS module whose imports will produce 'require' calls; however, the referenced file is an ECMAScript module and cannot be imported with 'require'. Consider writing a dynamic 'import("@remix-run/web-fetch")' call instead.

3 import { fetch as webFetch, Headers as WebHeaders, Request as WebRequest, Response as WebResponse } from "@remix-run/web-fetch";
                                                                                                           ~~~~~~~~~~~~~~~~~~~~~~

node_modules/@remix-run/node/dist/fetch.d.ts:4:26 - error TS1479: The current file is a CommonJS module whose imports will produce 'require' calls; however, the referenced file is an ECMAScript module and cannot be imported with 'require'. Consider writing a dynamic 'import("@remix-run/web-fetch")' call instead.

4 export { FormData } from "@remix-run/web-fetch";
                           ~~~~~~~~~~~~~~~~~~~~~~

node_modules/@remix-run/node/dist/fetch.d.ts:5:28 - error TS1479: The current file is a CommonJS module whose imports will produce 'require' calls; however, the referenced file is an ECMAScript module and cannot be imported with 'require'. Consider writing a dynamic 'import("@remix-run/web-file")' call instead.

5 export { File, Blob } from "@remix-run/web-file";
                             ~~~~~~~~~~~~~~~~~~~~~

node_modules/@remix-run/node/dist/fetch.d.ts:15:9 - error TS2416: Property 'headers' in type 'NodeRequest' is not assignable to the same property in base type 'Request'.
  Property 'getSetCookie' is missing in type 'import("/Users/cyrilchapon/Code/cyc/rfm-segments/node_modules/@remix-run/web-fetch/dist/src/headers", { assert: { "resolution-mode": "import" } }).default' but required in type 'Headers'.

15     get headers(): WebHeaders;
           ~~~~~~~

  node_modules/typescript/lib/lib.dom.d.ts:13447:5
    13447     getSetCookie(): string[];
              ~~~~~~~~~~~~~~~~~~~~~~~~~
    'getSetCookie' is declared here.

node_modules/@remix-run/node/dist/fetch.d.ts:16:5 - error TS2416: Property 'clone' in type 'NodeRequest' is not assignable to the same property in base type 'Request'.
  Type '() => NodeRequest' is not assignable to type '() => Request'.
    Call signature return types 'NodeRequest' and 'Request' are incompatible.
      The types of 'headers' are incompatible between these types.
        Type 'import("/Users/cyrilchapon/Code/cyc/rfm-segments/node_modules/@remix-run/web-fetch/dist/src/headers", { assert: { "resolution-mode": "import" } }).default' is not assignable to type 'Headers'.

16     clone(): NodeRequest;
       ~~~~~

node_modules/@remix-run/node/dist/fetch.d.ts:19:9 - error TS2416: Property 'headers' in type 'NodeResponse' is not assignable to the same property in base type 'Response'.
  Type 'import("/Users/cyrilchapon/Code/cyc/rfm-segments/node_modules/@remix-run/web-fetch/dist/src/headers", { assert: { "resolution-mode": "import" } }).default' is not assignable to type 'Headers'.

19     get headers(): WebHeaders;
           ~~~~~~~

node_modules/@remix-run/node/dist/fetch.d.ts:20:5 - error TS2416: Property 'clone' in type 'NodeResponse' is not assignable to the same property in base type 'Response'.
  Type '() => NodeResponse' is not assignable to type '() => Response'.
    Call signature return types 'NodeResponse' and 'Response' are incompatible.
      The types of 'headers' are incompatible between these types.
        Type 'import("/Users/cyrilchapon/Code/cyc/rfm-segments/node_modules/@remix-run/web-fetch/dist/src/headers", { assert: { "resolution-mode": "import" } }).default' is not assignable to type 'Headers'.

20     clone(): NodeResponse;
       ~~~~~

node_modules/@remix-run/node/dist/upload/fileUploadHandler.d.ts:47:22 - error TS2420: Class 'NodeOnDiskFile' incorrectly implements interface 'File'.
  Property 'prototype' is missing in type 'NodeOnDiskFile' but required in type 'File'.

47 export declare class NodeOnDiskFile implements File {
                        ~~~~~~~~~~~~~~

  node_modules/typescript/lib/lib.dom.d.ts:3124:5
    3124     prototype: Blob;
             ~~~~~~~~~
    'prototype' is declared here.

node_modules/@remix-run/web-fetch/dist/src/headers.d.ts:19:22 - error TS2420: Class 'import("/Users/cyrilchapon/Code/cyc/rfm-segments/node_modules/@remix-run/web-fetch/dist/src/headers", { assert: { "resolution-mode": "import" } }).default' incorrectly implements interface 'Headers'.
  Property 'getSetCookie' is missing in type 'import("/Users/cyrilchapon/Code/cyc/rfm-segments/node_modules/@remix-run/web-fetch/dist/src/headers", { assert: { "resolution-mode": "import" } }).default' but required in type 'Headers'.

19 export default class Headers extends URLSearchParams implements globalThis.Headers {
                        ~~~~~~~

  node_modules/typescript/lib/lib.dom.d.ts:13447:5
    13447     getSetCookie(): string[];
              ~~~~~~~~~~~~~~~~~~~~~~~~~
    'getSetCookie' is declared here.


Found 13 errors in 7 files.

Errors  Files
     1  app/entry.server.tsx:23
     1  app/routes/_index/route.tsx:6
     1  app/routes/app.tsx:3
     1  app/routes/auth.login/route.tsx:14
     7  node_modules/@remix-run/node/dist/fetch.d.ts:3
     1  node_modules/@remix-run/node/dist/upload/fileUploadHandler.d.ts:47
     1  node_modules/@remix-run/web-fetch/dist/src/headers.d.ts:19
error Command failed with exit code 2.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

Expected behavior

The doc states

Note: This template runs on JavaScript, but it's fully set up for TypeScript. If you want to create your routes using TypeScript, we recommend removing the noImplicitAny config from tsconfig.json

So I was expecting this to work with Typescript. And not only to compile with Remix, but also to be "easily fixable" when trying to typecheck — the whole point of Typescript not only being to transpile, but to typecheck in the first place.

Actual behavior

I'm getting typecheck errors

Steps to reproduce the problem

  1. Install the template
  2. Rename every .js to .ts, and every .jsx to .tsx
  3. Activate noImplicitAny: true in tsconfig.json
  4. Fix all implicit any errors (roughly, statically type every request / action / loader handlers parameters signature with types from Remix like LoaderArgs / Action Args / etc.
  5. Add a typecheck script "typecheck": "tsc"
  6. Go for yarn typecheck

Side note : (not trying to be spicy, but constructive) There has been a popular 1.5 years old issue about awful Typescript support in the Node template. As of 2023, the vanilla default is still a strange choice — and even more on a new template project like this one. Hard Typescript integration is exhausting, worrying, a even strict nogo for many developers in environments and frameworks. Furthermore, the whole underlying stack of React, Shopify app bridge, Polaris, Remix, etc. have first class Typescript support. What's the point here, seriously 🤨 ?

That argument there about "accessibility" of starters is IMHO a pure nonsense : creating Shopify apps involves deep fullstack development and multi-language understanding, often database + API knownledge, in-depth React — + 2 professional component frameworks — skills, GraphQL, OAuth, modern Node.js with not-that-simple architecture with Remix. That is not accessible by any means in the first place; and what we do need are professionnal tools 😬.

@paambaati
Copy link

After "activating" Typescript in a fresh install

@cyrilchapon Can you show me how to do this?

@patrickDouglas
Copy link

Once remix-run/remix#4371 (comment) this released, I think the template's versions can get bumped. That should fix all of the node_modules issues 🤞🏻. Also, do you have the remix.env.d.ts as described here? I think the *.css types are defined in @remix-run/dev.

@cyrilchapon
Copy link
Author

@paambaati

After "activating" Typescript in a fresh install

@cyrilchapon Can you show me how to do this?

I did provide a reproduction step


@patrickDouglas hum great to hear thanks !

  • Yes I did try the remix.env.d.ts, which basically caused more errors than without
  • Except indeed the .css which now fails

I'll watch the internal remix issue :)

@byrichardpowell
Copy link
Contributor

Hey @cyrilchapon

Thanks for opening an issue.

Seems like there is a mixture of things going on here

We definitely want to fix the issues with stuff that @shopify/shop-app-remix or shopify-app-template-remix provides. Sorry for these problems. To set expectations it might be a little down the list right now because we should prioritize some missing API's first.

As for providing a true Typescript template. Yeah, this pains me. Personally I love Typescript, and I dislike writing apps without it. We do want to prioritize a Typescript template so expect to see this land. Again, it might be a little while.

@cyrilchapon
Copy link
Author

cyrilchapon commented Aug 31, 2023

Hey @byrichardpowell ,

Honestly thanks for being so reinsuring here. The last @mkevinosullivan comment on the node template issue was actually frightening about Shopify care about typescript support for app developers.

Don't hesitate to reach, if needed. When pointed in the right direction, I can surely help on this.

@patrickDouglas
Copy link

The remix team is aware of another issue as well, which will solve the getSetCookie(): string[]; errors.

@byrichardpowell
Copy link
Contributor

byrichardpowell commented Oct 26, 2023

Hey everyone 👋

We've now released the Typescript version of the template. When you run yarn create @shopify/app and select Remix, you will prompted for either Typescript or JavaScript. Alternatively you can clone the template directly: https://github.com/Shopify/shopify-app-template-remix

The Javascript template is a separate branch here: https://github.com/Shopify/shopify-app-template-remix/tree/javascript

We will maintain the both and continue to improve the Typescript expereince. @paulomarg 's focus for example is Typescript type codegen for GraphQL queries. I won't scoop the work he's doing, but it's looking awesome and he'll share some stuff and ask for feedback soon. We'd love to hear what you think because we want to make the experience as good as possible.

Thank for your patience waiting for this.

Closing this issue as we now have a Typescript template. Please open a new issue for any Typescript issues you may discover.

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

4 participants