-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Setting up SDK workspace infrastructure #3505
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
base: main
Are you sure you want to change the base?
Conversation
| @@ -0,0 +1,9 @@ | |||
| { | |||
| // TODO: Eventually switch to user's config. | |||
| "extends": "../tsconfig.sdk.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.
tsconfig.sdk.json is our old sdk/wasp/tsconfig.json
| "skipLibCheck": true, | ||
| "allowJs": true, | ||
| "composite": true, | ||
|
|
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 removed rootDir and outDir fields.
Those are handled by core/tsconfig.json and user-core/tsconfig.json respectively.
This is because when using extends, the paths inside the extended tsconfig are calculated relatively to the extended tsconfig, not the tsconfig which extended it, which can be confusing.
E.g.
tsconfig.sdk.json has outDir: "."
core/tsconfig.json extends ../tsconfig.sdk.json
core/tsconfig.json will have outDir: ".." NOT outDir: "."
That's why I define them in referenced tsconfigs directly.
| While this is great, we still want to dig deeper at some point to understand | ||
| better why Vite HMR misbehaves when the SDK is recompiled: | ||
| https://github.com/wasp-lang/wasp/issues/1934 | ||
| */ |
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 also removed
"types": [
/*
This is needed to properly support Vitest testing with jest-dom matchers.
Types for jest-dom are not recognized automatically and Typescript complains
about missing types e.g. when using `toBeInTheDocument` and other matchers.
*/
"@testing-library/jest-dom"
],Because we no longer require it since moving jest-dom types to vite-end.d.ts file: https://github.com/wasp-lang/wasp/pull/2656/files
Full tested table:
vite - import '@testing-library/jest-dom' present in src/vite-env.d.ts
sdk - testing-library/jest-dom present in sdk/wasp/tsconfig.json's types
user - node_modules/@testing-library present in ./tsconfig.json's typeRoots of user's tsconfig
compiles - wasp compile
IDE - editor support, no "squiggly lines"
| vite | sdk | user | compiles | IDE | note |
|---|---|---|---|---|---|
| ❌ | ✅ | ✅ | ✅ | ✅ | ← state before #2656 PR |
| ✅ | ✅ | ❌ | ✅ | ✅ | ← current state |
| ✅ | ❌ | ❌ | ✅ | ✅ | ← the change I'm making |
| ❌ | ✅ | ❌ | ✅ | ❌ | runtime error (wasp start) |
| ❌ | ❌ | ❌ | ❌ | ❌ | |
| ❌ | ❌ | ✅ | ❌ | ✅ | |
| ✅ | ✅ | ✅ | ✅ | ✅ |
| sdkRootDirInGeneratedCodeDir :: Path' (Rel ProjectRootDir) (Dir SdkRootDir) | ||
| sdkRootDirInGeneratedCodeDir = [reldir|sdk/wasp|] | ||
| sdkRootDirInGeneratedCodeDir = [reldir|sdk/wasp/user-core|] | ||
|
|
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.
This solution mostly hardcodes paths to get the minimum changes to make a full vertical line work.
Making the new SDK paths infrastructure is for the next PR.
Deploying wasp-docs-on-main with
|
| Latest commit: |
d208fef
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://9d8903f7.wasp-docs-on-main.pages.dev |
| Branch Preview URL: | https://franjo-initial-workspaces.wasp-docs-on-main.pages.dev |
| export { type ServerSetupFn } from './types/index.js' | ||
| // PUBLIC API | ||
| export { HttpError } from './HttpError.js' | ||
| export { HttpError } from '../../core/server/HttpError.js' |
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.
Problem: package.json does not support the array options for exports field.
E.g., "./server": ["./dist/core/server/index.js", "./dist/user-core/server/index.js"], is not possible.
This means that when we split sdk/wasp into core and user-core tsconfig projects, we will need to:
- Re-export
coreexports in appropriateuser-corefiles.
Example isHttpErrorabove. - Change our
package.jsonto export./dist/user-core/<FILE>instead of./dist/<FILE>.
To make relative import/export paths work even in built files, we need to recreate the same structure they had in source files.
That's why we have:
sdk/wasp/
├── tsconfig.json
├── package.json
├── ...
├── core/
│ ├── tsconfig.json
│ └── ...
├── user-core/
│ ├── tsconfig.json
│ └── ...
└── dist/
├── core/
│ ├── .tsbuildinfo
│ └── ...
└── user-core/
├── .tsbuildinfo
└── ...
| "type": "module", | ||
| "scripts": { | ||
| "build": "tsc && node ./scripts/copy-assets.js", | ||
| "build": "tsc -b && node ./copy-assets.js", |
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.
-b flag is necessary to build referenced projects.
https://www.typescriptlang.org/docs/handbook/project-references.html#build-mode-for-typescript
| {=! Used by our code, undocumented (but accessible) for users. =} | ||
| "./core/serialization": "./dist/core/serialization/index.js", | ||
| "./core/auth": "./dist/core/auth.js", | ||
| "./core/serialization": "./dist/user-core/core/serialization/index.js", |
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.
How to handle exports once we actually split all of the files?
We will probably have 3 situations:
-
Old
sdk/wasp/file is moved fully tocore/
Probably will point to./dist/core/or we can still force the exports through./dist/user-core/. -
Old
sdk/wasp/file is moved fully touser-core/
Probably will point to./dist/user-core/ -
Old
sdk/wasp/file is partially moved tocore/and partially moved touser-core/
Probably will re-exportcore/stuff inuser-core/and point to./dist/user-core.
Like withHttpErrorabove.
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.
Maybe a separate project/dir for public API?
Research!
| -- We are using the same SDK location for both build and start. Read this issue | ||
| -- for the full story: https://github.com/wasp-lang/wasp/issues/1769 | ||
| let sdkDir = waspProjectDir </> dotWaspDirInWaspProjectDir </> generatedCodeDirInDotWaspDir </> sdkRootDirInProjectRootDir | ||
| let sdkDir = waspProjectDir </> dotWaspDirInWaspProjectDir </> generatedCodeDirInDotWaspDir </> realSdkRootDirInGeneratedCodeDir |
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.
Since I made sdkRootDirInProjectRootDir have a value of sdk/wasp/user-core to minimize changes for for SDK template files, the build wasn't properly copying as I need to copy the root dir.
realSdkRootDirInGeneratedCodeDir instead points to sdk/wasp which makes it work.
@wasp.sh/wasp-cli
@wasp.sh/wasp-cli-darwin-arm64-unknown
@wasp.sh/wasp-cli-darwin-x64-unknown
@wasp.sh/wasp-cli-linux-x64-glibc
@wasp.sh/wasp-cli-linux-x64-musl
commit: |
| @@ -1,6 +1,6 @@ | |||
| import { useContext } from 'react' | |||
| import { useForm } from 'react-hook-form' | |||
| import { requestPasswordReset } from '../../../email/actions/passwordReset.js' | |||
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.
When I moved the whole folder, VSC organized all imports automatically for the moved files.
I tried to minimize the large changes (sorting) and left ones which I deem more beneficial:
- removal of
.jsxor.jssuffixes - unused imports
The rest were reverted to best of my abilities.
We can discuss if we want to revert all of them.
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.
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.
Should be okay because of: #2493
We still don't bundle SDK individually, but through the server.
| </> sdkRootDirInGeneratedCodeDir | ||
|
|
||
| realSdkRootDirInGeneratedCodeDir :: Path' (Rel ProjectRootDir) (Dir SdkRootDir) | ||
| realSdkRootDirInGeneratedCodeDir = [reldir|sdk/wasp|] |
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.
Used only for wasp build command.
Description
Part of issue: #2247
More specifically, part 1 of: #2247 (comment)
Confirmed working:
examples/kitchen-sinkwasp compile✅wasp start✅wasp build✅wasp build start✅Type of change
Checklist
I tested my change in a Wasp app to verify that it works as intended.
🧪 Tests and apps:
examples/kitchen-sink/e2e-tests.waspc/data/Cli/templates, as needed.examples/, as needed.examples/tutorials) I updated the tutorial in the docs (and vice versa).📜 Documentation:
web/docs/.🆕 Changelog: (if change is more than just code/docs improvement)
waspc/ChangeLog.mdwith a user-friendly description of the change.web/docs/migration-guides/.versioninwaspc/waspc.cabalto reflect the changes I introduced.