TheEdoRan / next-safe-action

Type safe and validated Server Actions in your Next.js project.
https://next-safe-action.dev
MIT License
2.23k stars 35 forks source link

Is this a turborepo issue? #64

Closed dugan-dev closed 1 month ago

dugan-dev commented 8 months ago

Is there an existing issue for this?

Library version (optional)

6.1.0

Ask a question

I've been using next-safe-action since v3 and think this is a great tool. Please keep up the great work!

I recently migrated to a turborepo mono repo and am using pnpm as the package manager. In VSCode when looking at the safe-action file or the safe actions themselves code and when I try to build I get this error.

vbpay:build: Type error: The inferred type of 'action' cannot be named without a reference to '../../node_modules/next-safe-action/dist/index-7AaVMS6R.mjs'. This is likely not portable. A type annotation is necessary. nextjs:build: nextjs:build: 2 | import { createSafeActionClient } from "next-safe-action"; nextjs:build: 3 | nextjs:build: > 4 | export const action = createSafeActionClient({ nextjs:build: | ^ nextjs:build: 5 | async middleware() { nextjs:build: 6 | const { userId } = auth(); nextjs:build: 7 | if (userId === null) { nextjs:build:  ELIFECYCLE  Command failed with exit code 1.

When I open ../../node_modules/next-safe-action/dist/index-7AaVMS6R.mjs I saw it had an error bc it was importing @decs/typeschema which wasn't installed so i installed that but still getting the same error.

The funny thing is everything works perfectly fine when I run in dev mode. Of course, if I have a type mismatch between my action schema and type im passing in its brutal because of these errors the typing doesn't work for me to know if its a real error or not.

Additional context

I thought maybe it was pnpm issue but I get the same error when using npm as the package manager for the turbo repo mono repo.

TheEdoRan commented 8 months ago

Hi @dugan-dev, it could be. Can you please provide a link to a repo with a minimal reproducible example of this issue? Thanks!

dugan-dev commented 8 months ago

@TheEdoRan As requested here a link to a repo with the issue.

https://github.com/dugan-dev/safe-action-issue-reproduction

TheEdoRan commented 7 months ago

Sorry for the delay and thank you for providing a reproduction link. This is a very tricky one, and I still have to find a way to properly solve it. This problem affects multiple projects, and I am unsure if in this case it's caused by tsup bundling, a TypeScript config, or simply using pnpm in a monorepo. It's related (at least) to these TypeScript issues, and a number of others in multiple repos:

As a workaround, suggested by some users, setting these two options in tsconfig.json seem to work, but it's certainly not that great of a solution, and if you need to emit declaration files when building the project you can't use them. Anyways, here they are:

{
  "compilerOptions": {
    "declaration": false,
    "declarationMap": false,
  }
}

Let me know if this works for you as well, thanks.

dugan-dev commented 7 months ago

@TheEdoRan This solved my problem. Thank you so much!

nikitavoloboev commented 5 months ago

Getting same error in Vercel:

image

Solution with

{
  "compilerOptions": {
    "declaration": false,
    "declarationMap": false,
  }
}

Does not work, tsconfig: https://github.com/kuskusapp/kuskus/blob/main/tsconfig.json

ilanyehez commented 2 months ago

I'm also having this problem when trying to upgrade to v7. When using a monorepo it is essential to use type declarations. There must be another way to fix this.

@TheEdoRan I've found a temp solution, if you explicitly set the type of the store the error goes away. The only problem is that there isn't an exported type of the store so I have to do something like this, it works but we lose automatic types from the action and ctx:

export type SafeActionClient = ReturnType<
  typeof createSafeActionClient<DVES, string, MetadataSchema>
>
export const publicAction: SafeActionClient = createSafeActionClient<DVES, string, MetadataSchema>({
  handleReturnedServerError(e) {
    if (e instanceof ActionError) {
      return e.message
    }

    return DEFAULT_SERVER_ERROR_MESSAGE
  },
  })
TheEdoRan commented 2 months ago

@ilanyehez GitHub did not notify me after you edited your comment with the tag, so sorry for my late reply. By the way, thanks for finding that out! I'm going to look into it soon.

TheEdoRan commented 2 months ago

@ilanyehez please try to install next-safe-action v7.7.1-beta.1 with:

npm i next-safe-action@beta

and let me know if it fixes your issue, thanks. Now the return type of createSafeActionClient is explicitly set and not inferred.

ilanyehez commented 2 months ago

Thank you for the fast response :)

I've installed the beta but still get the same error:

The inferred type of 'publicAction' cannot be named without a reference to '../../../node_modules/next-safe-action/dist/index.types-C7qSUCH-.mjs'. This is likely not portable. A type annotation is necessary.ts(2742)
const publicAction: SafeActionClient<string, undefined, undefined, undefined, {}, undefined, undefined, readonly [], undefined, undefined>

I was previously on 7.6.4 when my type workaround worked, and now my workaround doesn't work, typescript has more errors...

TheEdoRan commented 2 months ago

@ilanyehez yeah, you're right. Right now I'm testing with an actual monorepo set up with pnpm and I've just found out that setting compilerOptions.baseUrl to "." in the package's tsconfig.json solves the typing issue, without needing to deal with declaration settings. If this is the case for you as well, do you think it would be a viable solution?

Source:

ilanyehez commented 2 months ago

@TheEdoRan I already have baseUrl set to ".", this is my main setup for packages:

 "composite": true, // Enables project references and informs the TypeScript program where to find referenced outputs.
    "declaration": true, // Project references rely on the compiled declarations (.d.ts) of external projects. If declarations do not exist, TypeScript will generate them on demand.
    "declarationMap": true, // Generate sourcemaps for declarations, so that language server integrations in editors like "Go to" resolve correctly.
    "incremental": true, // Enables incremental compilation, greatly improving performance.
    "noEmitOnError": true, // If the typechecker fails, avoid generating invalid or partial declarations.
    "skipLibCheck": true, // Avoids eager loading and analyzing all declarations, greatly improving performance.
    "verbatimModuleSyntax": false, // Avoids generating verbose module syntax in declarations.

    // proper ESM support:
    "allowSyntheticDefaultImports": true,
    "esModuleInterop": true,
    "isolatedModules": true,
    "moduleResolution": "bundler",
    "module": "esnext",
    "strict": true,
    "strictNullChecks": true,
    "target": "esnext",

    // react support
    "allowJs": true,
    "jsx": "preserve",
    "lib": ["dom", "dom.iterable", "esnext"],
    "resolveJsonModule": true

     "emitDeclarationOnly": true,
    "noImplicitAny": false,
    "outDir": "./lib",
    "baseUrl": ".",

    // and more specific settings for paths and references
    ...paths
    ...references
TheEdoRan commented 2 months ago

@ilanyehez is "baseUrl": "." set in the actual package/app tsconfig.json? Because if so it's pretty weird, the moment I added that setting to my package's tsconfig.json (not the root one or shared ones), the error went away instantly. If it doesn't work for you can you please provide a link to a repo with a minimal reproduction of the issue? For reference, I used this one for tests:

ilanyehez commented 2 months ago

@TheEdoRan Yes, the baseUrl is individually set for each package or app.

The most comprehensive guide about setting up typescript on a monorepo can be found here, maybe it'll give you an idea: https://moonrepo.dev/docs/guides/javascript/typescript-project-refs#configuration

github-actions[bot] commented 1 month ago

:tada: This issue has been resolved in version 7.9.3 :tada:

The release is available on:

Your semantic-release bot :package::rocket: