bugfix: Upgrade to Saleor App SDK v1.3.0 and improve APL configuration#28
bugfix: Upgrade to Saleor App SDK v1.3.0 and improve APL configuration#28sultan18kh wants to merge 3 commits intosaleor:mainfrom
Conversation
- Upgrade @saleor/app-sdk from 0.50.1 to 1.3.0 with breaking changes - Update APL imports to use individual modules (file, saleor-cloud, upstash) - Add proper environment variable validation for Upstash APL configuration - Add metadata deletion functionality to metadata manager - Update tRPC and webhook implementations for SDK compatibility - Update dependencies: next, graphql, zod, tsx, vite, vitest - Add comprehensive environment configuration documentation (env.example)
feat: upgrade to Saleor App SDK v1.3.0 and improve APL configuration
|
Sultan Mustafa Khan seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
|
Sultan Mustafa Khan seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
|
@witoszekdev, @lkostrowski, secondary email has been added, but still the CLA status hasn't been updated. Kindly verify. Thanks! |
| // TODO: JWT verification is temporarily disabled due to missing verifyJWT function | ||
| // try { | ||
| // logger.debug("trying to verify JWT token from frontend"); | ||
| // logger.debug({ token: ctx.token ? `${ctx.token[0]}...` : undefined }); | ||
|
|
||
| // await verifyJWT({ | ||
| // appId: ctx.appId, | ||
| // token: ctx.token, | ||
| // saleorApiUrl: ctx.saleorApiUrl, | ||
| // requiredPermissions: [ | ||
| // ...REQUIRED_SALEOR_PERMISSIONS, | ||
| // ...(meta?.requiredClientPermissions || []), | ||
| // ], | ||
| // }); | ||
| // } catch (e) { | ||
| // logger.debug("JWT verification failed, throwing"); | ||
| // throw new TRPCError({ | ||
| // code: "UNAUTHORIZED", | ||
| // message: "JWT verification failed", | ||
| // }); | ||
| // } |
There was a problem hiding this comment.
issue: we cannot merge this with missing JWT validation. It looks like there shouldn't be any issue here, since we also use this method in other Saleor apps. Example avatax app: https://github.com/saleor/apps/blob/3935a8bec907e05e94287bf330a4bd3b4fbe7849/apps/avatax/src/modules/trpc/protected-client-procedure.ts?plain=1#L1 (it uses app sdk v1.3.0)
Could you please add this back?
| # App URLs and Base Configuration | ||
| # =============================== | ||
|
|
||
| # Base URL for your app (used for manifest and webhooks) | ||
| # In development, this is typically http://localhost:3000 | ||
| # In production, this should be your deployed app URL | ||
| APP_BASE_URL=http://localhost:3000 | ||
|
|
||
| # Optional: Override iframe base URL (for embedded app interface) | ||
| # If not set, uses APP_BASE_URL | ||
| APP_IFRAME_BASE_URL=http://localhost:3000 | ||
|
|
||
| # Optional: Override API base URL (for webhooks and API endpoints) | ||
| # If not set, uses APP_BASE_URL | ||
| APP_API_BASE_URL=http://localhost:3000 |
There was a problem hiding this comment.
suggestion: These are actually not required to run the app. It's used when you need to have app work with Saleor running in a Docker container for example
It's described here in our docs: https://docs.saleor.io/developer/extending/apps/local-app-development#apps-url-overriding
I'm worried this description suggests you must provide this, which would be incorrect: it's used only for development, if you would use these env variables on production app it wouldn't have any multitenancy, since it would always try to connect to the same Saleor instance.
| # 1. For local development, you only need: | ||
| # - SECRET_KEY (required) | ||
| # - APP_BASE_URL (optional, defaults to http://localhost:3000) | ||
| # - APL=file (default) |
There was a problem hiding this comment.
suggestion: I think if we only need these 3 we should probably have only these variables commented out, rest should be commented to avoid accidentaly using them
| # 3. For Saleor Cloud deployment: | ||
| # - Set APL=rest | ||
| # - Configure REST_APL_ENDPOINT and REST_APL_TOKEN | ||
| # - Set SALEOR_CLOUD_TOKEN and SALEOR_CLOUD_RESOURCE_URL for migrations |
There was a problem hiding this comment.
suggestion: could you please remove this part? 🙏🏻 we don't need instructions for Saleor Cloud deployment in example 😉
3 participants
Upgrade to Saleor App SDK v1.3.0 and improve APL configuration