-
Notifications
You must be signed in to change notification settings - Fork 21
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
feat: implement shared zod schema between backend and frontend #1300
Conversation
alias: [{ find: '@', replacement: resolve(__dirname, 'src') }] | ||
alias: { | ||
'@': resolve(__dirname, 'src'), | ||
'@shared/boutique': resolve(__dirname, '../../boutique/shared/src') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to have the type alias if @shared/boutique
is a package.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well without this we will get Failed to resolve entry for package "@boutique/shared"
error in locally run build by docker
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've looked into this. The main reason behind this error is that vite
is expecting the source code to be ESM only. and we are bundling @boutique/shared
as CJS. This should be okay for now, but I think we should discuss if we want to move everything to ESM. A lot of libraries are starting to be bundled for ESM only.
Node 22 will have support for importing ESM into CJS using require
, but we will be able to use this functionality in Octomber/November.
- https://nodejs.org/en/blog/announcements/v22-release-announce#support-requireing-synchronous-esm-graphs
- https://nodejs.org/en/about/previous-releases
Converting the shared package to ESM will throw a runtime error when boutique's backend is ran:
Error [ERR_REQUIRE_ESM]: require() of ES Module /Users/radu/dev/testnet/packages/boutique/shared/dist/index.js from /Users/radu/dev/testnet/packages/boutique/backend/dist/order/controller.js not supported.
Instead change the require of index.js in /Users/radu/dev/testnet/packages/boutique/backend/dist/order/controller.js to a dynamic import() which is available in all CommonJS modules.
at Object.<anonymous> (/Users/radu/dev/testnet/packages/boutique/backend/dist/order/controller.js:7:18) {
code: 'ERR_REQUIRE_ESM'
}
packages/boutique/shared/index.ts
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move index.ts
in src
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I do that it causes changes in all imports import {schema} from '@shared/boutique/src'
because of this, I create index.ts
out of src
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've put up a PR that takes cares of this: #1343.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# Conflicts: # packages/boutique/frontend/package.json
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is the structure we're aiming for:
-
a shared package at the same level with wallet and boutique, that has backend shared functionality.
-
another shared directory both in wallet and boutique, which basically holds API types / schema.
I'd change the 'shared' directory name in wallet and boutique to 'models', so there is less naming confusions.
thoughts?
* Update package.json and move `index.ts` to `src` * Fix lockfile
# Conflicts: # pnpm-lock.yaml
…tique # Conflicts: # pnpm-lock.yaml
Context
Changes