analogjs / analog

The fullstack meta-framework for Angular. Powered by Vite and Nitro
https://analogjs.org
MIT License
2.59k stars 249 forks source link

New Analog App (nx v17) fails to build - trpc issue? #722

Closed Pascalmh closed 11 months ago

Pascalmh commented 1 year ago

Please provide the environment you discovered this bug in.

I tried installing Analog using the NX preset on:

My Macbook and on gitpod.io - same results.

Which area/package is the issue in?

create-analog

Description

After creating a new NX Workspace with the Analog preset the build fails.

Please provide the exception or error you saw

$ npx create-nx-workspace@latest --preset=@analogjs/platform                                                                                                 

 >  NX   Let's create a new workspace [https://nx.dev/getting-started/intro]

✔ Where would you like to create your workspace? · my-org
✔ Enable distributed caching to make your CI faster · Yes

 >  NX   Creating your v17.0.1 workspace.

   To make sure the command works reliably in all environments, and that the preset is applied correctly,
   Nx will run "npm install" several times. Please wait.

✔ Installing dependencies with npm
✔ Successfully created the workspace: my-org.
✔ What name would you like to use for your Analog app? · analog-app
✔ Add TailwindCSS for styling? (Y/n) · true
✔ Add tRPC for typesafe client/server interaction? (y/N) · true
Angular has not been installed yet. Creating an Angular application
Fetching @nx/angular...
Fetching @angular-devkit/core...
Fetching prettier...
✔ NxCloud has been set up successfully

 >  NX   Distributed caching via Nx Cloud has been enabled

   In addition to the caching, Nx Cloud provides config-free distributed execution,
   UI for viewing complex runs and GitHub integration. Learn more at https://nx.app

   Your workspace is currently unclaimed. Run details from unclaimed workspaces can be viewed on cloud.nx.app by anyone
   with the link. Claim your workspace at the following link to restrict access.

   https://cloud.nx.app/orgs/workspace-setup?accessToken=[my-access-token]

 >  NX   Successfully applied preset: @analogjs/platform

$ npx nx build analog-app                                                                                                                        

> nx run analog-app:build:production

node_modules/@trpc/client/src/TRPCClientError.ts:42:18 - error TS4111: Property 'error' comes from an index signature, so it must be accessed with ['error'].

  40 |   return (
  41 |     isObject(obj) &&
> 42 |     isObject(obj.error) &&
     |                  ^
  43 |     typeof obj.error.code === 'number' &&
  44 |     typeof obj.error.message === 'string'
  45 |   );

node_modules/@trpc/client/src/TRPCClientError.ts:43:16 - error TS4111: Property 'error' comes from an index signature, so it must be accessed with ['error'].

  41 |     isObject(obj) &&
  42 |     isObject(obj.error) &&
> 43 |     typeof obj.error.code === 'number' &&
     |                ^
  44 |     typeof obj.error.message === 'string'
  45 |   );
  46 | }

node_modules/@trpc/client/src/TRPCClientError.ts:43:22 - error TS4111: Property 'code' comes from an index signature, so it must be accessed with ['code'].

  41 |     isObject(obj) &&
  42 |     isObject(obj.error) &&
> 43 |     typeof obj.error.code === 'number' &&
     |                      ^
  44 |     typeof obj.error.message === 'string'
  45 |   );
  46 | }

node_modules/@trpc/client/src/TRPCClientError.ts:44:16 - error TS4111: Property 'error' comes from an index signature, so it must be accessed with ['error'].

  42 |     isObject(obj.error) &&
  43 |     typeof obj.error.code === 'number' &&
> 44 |     typeof obj.error.message === 'string'
     |                ^
  45 |   );
  46 | }
  47 | 

node_modules/@trpc/client/src/TRPCClientError.ts:44:22 - error TS4111: Property 'message' comes from an index signature, so it must be accessed with ['message'].

  42 |     isObject(obj.error) &&
  43 |     typeof obj.error.code === 'number' &&
> 44 |     typeof obj.error.message === 'string'
     |                      ^
  45 |   );
  46 | }
  47 | 

node_modules/@trpc/client/src/TRPCClientError.ts:103:7 - error TS2322: Type 'TRPCClientError<ErrorInferrable>' is not assignable to type 'TRPCClientError<TRouterOrProcedure>'.
  Type 'ErrorInferrable' is not assignable to type 'TRouterOrProcedure'.
    'ErrorInferrable' is assignable to the constraint of type 'TRouterOrProcedure', but 'TRouterOrProcedure' could be instantiated with a different subtype of constraint 'ErrorInferrable'.
      Type 'AnyProcedure' is not assignable to type 'TRouterOrProcedure'.
        'AnyProcedure' is assignable to the constraint of type 'TRouterOrProcedure', but 'TRouterOrProcedure' could be instantiated with a different subtype of constraint 'ErrorInferrable'.

  101 |     }
  102 |     if (isTRPCErrorResponse(cause)) {
> 103 |       return new TRPCClientError(cause.error.message, {
      |       ^
  104 |         ...opts,
  105 |         result: cause,
  106 |       });

Found 6 errors.

 >  NX   Found type errors. See above.

   Pass --verbose to see the stacktrace.

 ———————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————

 >  NX   Ran target build for project analog-app (3s)

    ✖    1/1 failed
    ✔    0/1 succeeded [0 read from cache]

Other information

Adding the --verbose flag doesn't provide more information.

Setting noPropertyAccessFromIndexSignature to false in the tsconfig.json removes the TS4111-Errors.

I would be willing to submit a PR to fix this issue

brandonroberts commented 1 year ago

@goetzrobin ran into this also, the workaround for now is to set skipTypeCheck: true in the build executor options. What's the proposed fix as this is coming from the tRPC library?

goetzrobin commented 1 year ago

@brandonroberts @Pascalmh I did some more digging and for some reason the trpc client and server packages include a src folder with .ts files. Deleting the folder manually allows me to build with the default setup...

I am trying to figure out why that folder is there. I'll see if I can find out what is going on

goetzrobin commented 1 year ago

Weirdly enough I tried to see if I can replicate the issue with the @nx/vite builder and tRPC in a simple nx react application and there I am not getting the same issue... Which makes me wonder what config is causing it to type check node_modules 🤔

enisze commented 11 months ago

any news regarding this issue? I got the same trpc error on my next /react application is there a corresponding issue on trpc side?

goetzrobin commented 11 months ago

@enisze that's interesting! That makes me think that this is a conflict between trpc & some typescript configuration... are you using Nx?

enisze commented 11 months ago

hey @goetzrobin no I am not using nx

homj commented 11 months ago

I'm seeing this issue as well after following https://www.spartan.ng/stack/installation

jeremyhofer commented 11 months ago

I've been digging into this today. I've narrowed down some of the issues.

The Property 'error' comes from an index signature, so it must be accessed with ['error']. error being thrown specifically is happening due to noPropertyAccessFromIndexSignature being set to true in the app's tsconfig.json. Setting to false clears these errors.

This is occurring now as the code in question was just recently introduced in this PR in trpc: https://github.com/trpc/trpc/pull/4796

The above change was released in trpc v10.38.3: https://github.com/trpc/trpc/releases/tag/v10.38.3

Installing @trpc/client and @trpc/server version 10.38.2 doesn't have this issue, confirming an issue with the change above.

Similarly, the Type 'TRPCClientError<ErrorInferrable>' is not assignable to type 'TRPCClientError<TRouterOrProcedure>'. error is also introduced via the above PR. However, it is a bit more complex.

Testing, trpc has no issue compiling the client despite that type error / issue. Experimenting and checking typescript versions, this seems to be due to a type narrowing regression specifically in typescript 5.1.3, which trpc is currently built with. Downgrading to an earlier 5.1.x version before the regression or upgrading to a newer 5.1.x version after the regression was fixed causes trpc to fail to build.

I didn't see any related issues reported on trpc's issues board, so I just created one: https://github.com/trpc/trpc/issues/5175

KATT commented 11 months ago

Enable skipLibCheck, it also helps ts perf

That said, we should still fix these in trpc

jeremyhofer commented 11 months ago

@KATT in this case skipLibCheck is true , but not having an impact during the build. From @goetzrobin 's earlier comment skipLibCheck seems to be running due to the full trpc source being included in the npm packages. According to the typescript docs skipLibCheck will only specifically ignore *.d.ts files: https://www.typescriptlang.org/tsconfig#skipLibCheck

goetzrobin commented 11 months ago

I wonder if this could have something to do with the tsconfig path plugin. If you use the same tRPC in a new Nx workspace with React, do we get the same issue? I'll have to test that.

KATT commented 11 months ago

@jeremyhofer it would still typecheck tRPC even it if it was only .d.ts-files. We ship the TS files with declarationMap so that people can CMD+Click into tRPC source code [and hopefully help with features and bug issues].

I'd say most projects should use skipLibChecks, for a boilerplate it makes sense; otherwise, you'll play whack-a-mole on dependencies of your project forever. Also, as previously mentioned, it improves TS perf.

By default, TypeScript performs a full re-check of all .d.ts files in a project to find issues and inconsistencies; however, this is typically unnecessary. [...]

https://github.com/microsoft/TypeScript/wiki/Performance#skipping-dts-checking

(This has been my stance/default for the last few years, but I might be wrong and if I am I'd love to be educated)


On compilation, as far as I know, the TS compiler will ignore it if skipLibCheck is true so it might be something specific to Nx/analog

jeremyhofer commented 11 months ago

Thanks for the additional context Katt.

I just raised #805 which looks to fix this. Some of the imports here in analog were importing directly from the src/ folders of the trpc packages, which was causing skipLibChecks to not be respected. I updated these imports to roll them into the standard library imports instead (minus one import into the dist/ folder in one of the libs).

Testing locally this seems to fix the build issues from the analog perspective. @goetzrobin can you take a look and help test the import changes?

goetzrobin commented 11 months ago

@jeremyhofer thanks so much! I'll check this out first thing tomorrow! We do have the e2e test-app for trpc that verifies core functionality! If that still passes it should be good! Great work! I think that was one of the major issues of the trpc story...