-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
TS SDK Setup #2299
TS SDK Setup #2299
Changes from 13 commits
8a4f788
ce3f118
4d7d8e7
f14b617
2083bdb
40dbc78
934e8ae
384116a
f81a3e8
01fc493
30621f5
30c3754
ccfd371
2f9981d
96f7bba
098d0c6
eddebd1
904b8ce
5041f55
acbd407
a77a7b0
eb44164
a91b88a
5894a58
55690ae
21cc896
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,6 +32,7 @@ import qualified Wasp.Cli.Command.Start.Db as Command.Start.Db | |
import Wasp.Cli.Command.Studio (studio) | ||
import qualified Wasp.Cli.Command.Telemetry as Telemetry | ||
import Wasp.Cli.Command.Test (test) | ||
import Wasp.Cli.Command.TsConfigSetup (tsConfigSetup) | ||
import Wasp.Cli.Command.Uninstall (uninstall) | ||
import Wasp.Cli.Command.WaspLS (runWaspLS) | ||
import Wasp.Cli.Message (cliSendMessage) | ||
|
@@ -51,6 +52,7 @@ main = withUtf8 . (`E.catch` handleInternalErrors) $ do | |
["start"] -> Command.Call.Start | ||
["start", "db"] -> Command.Call.StartDb | ||
["clean"] -> Command.Call.Clean | ||
["ts-setup"] -> Command.Call.TsSetup | ||
["compile"] -> Command.Call.Compile | ||
("db" : dbArgs) -> Command.Call.Db dbArgs | ||
["uninstall"] -> Command.Call.Uninstall | ||
|
@@ -102,6 +104,7 @@ main = withUtf8 . (`E.catch` handleInternalErrors) $ do | |
Command.Call.Start -> runCommand start | ||
Command.Call.StartDb -> runCommand Command.Start.Db.start | ||
Command.Call.Clean -> runCommand clean | ||
Command.Call.TsSetup -> runCommand tsConfigSetup | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Btw, we need to come up with a proper common name for the TS SDK. TS SDK isn't ideal because we already call our I called the TS SDK package In Haskell though, I call it There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yup I agree, TS SDK is not the best name. Config certainly seems to be one of the keywords that makes sense. These are config files. They are wasp config files. In Wasp DSL, or in TS. Or maybe one could just call them Wasp files? Hm. Wasp spec files. One question is, what does the import look like for it in Then the question is how do we call this in general in front of users. Maybe "Wasp TS spec" or "Wasp TS config" vs "Wasp DSL spec" or "Wasp DSL config". Yeah. And then it comes down to how we call it internally, in our code. But I think we will figure that out once we figured out the stuff above. I don't think we have to figure this out right now, let's just go with the nomenclature you used so far for this experimental version, but then after we release it let's talk and figure out the final naming. Maybe you can add that to your TODO list for TS SDK. |
||
Command.Call.Compile -> runCommand compile | ||
Command.Call.Db dbArgs -> dbCli dbArgs | ||
Command.Call.Version -> printVersion | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,6 +7,7 @@ data Call | |
| StartDb | ||
| Clean | ||
| Uninstall | ||
| TsSetup | ||
| Compile | ||
| Db Arguments -- db args | ||
| Build | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,6 +9,7 @@ import qualified Data.Text as T | |
import StrongPath (Abs, Dir, File, Path') | ||
import Wasp.Cli.Command.CreateNewProject.Common (defaultWaspVersionBounds) | ||
import Wasp.Cli.Command.CreateNewProject.ProjectDescription (NewProjectAppName, NewProjectName) | ||
import Wasp.NodePackageFFI (InstallablePackage (WaspConfigPackage), getPackageInstallationPath) | ||
import Wasp.Project.Analyze (WaspFile (..), findWaspFile) | ||
import Wasp.Project.Common (WaspProjectDir) | ||
import Wasp.Project.ExternalConfig.PackageJson (findPackageJsonFile) | ||
|
@@ -43,8 +44,16 @@ replaceTemplatePlaceholdersInPackageJsonFile appName projectName projectDir = | |
Just absPackageJsonFile -> replaceTemplatePlaceholdersInFileOnDisk appName projectName absPackageJsonFile | ||
|
||
replaceTemplatePlaceholdersInFileOnDisk :: NewProjectAppName -> NewProjectName -> Path' Abs (File f) -> IO () | ||
replaceTemplatePlaceholdersInFileOnDisk appName projectName = | ||
updateFileContentWith (replacePlaceholders waspTemplateReplacements) | ||
replaceTemplatePlaceholdersInFileOnDisk appName projectName file = do | ||
waspConfigPackagePath <- getPackageInstallationPath WaspConfigPackage | ||
let waspTemplateReplacements = | ||
[ ("__waspConfigPath__", waspConfigPackagePath), | ||
("__waspAppName__", show appName), | ||
("__waspProjectName__", show projectName), | ||
("__waspVersion__", defaultWaspVersionBounds) | ||
] | ||
-- TODO: We do this in all files, but not all files have all placeholders | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This bothers me a little, but I guess it's no big deal? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But this how it was before also, right? Yeah, that is fine then. Btw this is afunction that is called upon a file, logic that decided which files this will be called upon is not in this function, so this comment should be referencing some other peice of code I guess, that I don't see from here at the moment. |
||
updateFileContentWith (replacePlaceholders waspTemplateReplacements) file | ||
where | ||
updateFileContentWith :: (Text -> Text) -> Path' Abs (File f) -> IO () | ||
updateFileContentWith updateFn absFilePath = IOUtil.readFileStrict absFilePath >>= IOUtil.writeFileFromText absFilePath . updateFn | ||
|
@@ -53,9 +62,3 @@ replaceTemplatePlaceholdersInFileOnDisk appName projectName = | |
replacePlaceholders replacements content = foldl' replacePlaceholder content replacements | ||
where | ||
replacePlaceholder content' (placeholder, value) = T.replace (T.pack placeholder) (T.pack value) content' | ||
|
||
waspTemplateReplacements = | ||
[ ("__waspAppName__", show appName), | ||
("__waspProjectName__", show projectName), | ||
("__waspVersion__", defaultWaspVersionBounds) | ||
] |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
module Wasp.Cli.Command.TsConfigSetup (tsConfigSetup) where | ||
|
||
import Control.Concurrent (newChan) | ||
import Control.Monad.IO.Class (liftIO) | ||
import Wasp.Cli.Command (Command, require) | ||
import Wasp.Cli.Command.Require (InWaspProject (InWaspProject)) | ||
import Wasp.Generator.NpmInstall (installProjectNpmDependencies) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A bit wild, importing this from the Generator. Maybe it should not be in the Generator? or we should not be calling it here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I will add a comment. |
||
|
||
-- | Prepares the project for using Wasp's TypeScript SDK. | ||
tsConfigSetup :: Command () | ||
tsConfigSetup = do | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This method is still unfinished. It's supposed to set up a project to use the TS SDK but currently only does half the work (check the TODOs). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok so this method doesn't do anything but install npm deps? Which npm deps, just for the "wasp project", not for the generated code right? So stuff like prisma, and I guess this new wasp-config? Or how does this go, I don't have the full idea of execution flow here. What else coudl it do: create missing ts config files, inject stuff into package json, ... ? Is this only temporary? Yeah hm this is connected to my comment above where I am trying to understand how we imagine in the future what is this setup going to look like, and also what is needed now by them, for this experimental version, to be able to use it. |
||
InWaspProject waspProjectDir <- require | ||
messageChan <- liftIO newChan | ||
-- TODO: Both of these should be eaiser when Miho finishes the package.json | ||
-- and tsconfig.json validation: | ||
-- - Edit package.json to contain the SDK package | ||
-- - Adapt TSconfigs to fit with the TS SDK project structure | ||
liftIO $ | ||
-- NOTE: We're only installing the user's package.json dependencies here | ||
-- This is to provide proper IDE support for users working with the TS SDK | ||
-- (it needs the `wasp-config` package). | ||
-- Calling this function here shouldn't break anything for later | ||
-- installations. | ||
-- TODO: What about doing this during Wasp start? Can we make Wasp start | ||
-- pick up whether the user wants to use the TS SDK automatically? | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is an idea for the future, not for now. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't have a good enough grasp of the whole exeuction flow to answer this, but it does sound like we want this to happen automatically, for sure. I could probably discuss this better if I understood the pros / cons / alternatives / exec flow. |
||
installProjectNpmDependencies messageChan waspProjectDir >>= \case | ||
Left e -> putStrLn $ "npm install failed: " ++ show e | ||
Right _ -> return () |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,7 @@ | |
"react": "^18.2.0" | ||
}, | ||
"devDependencies": { | ||
"wasp-config": "file:__waspConfigPath__", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh ok, so new templates will be coming with this! So even if I am not using wasp-config, I will have this in my template. Not ideal for experimental feature hm, as it will kind of "polute" their package.json. |
||
"typescript": "^5.1.0", | ||
"vite": "^4.3.9", | ||
"@types/react": "^18.0.37", | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
node_modules/ | ||
dist/ | ||
.env | ||
.vscode/ |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
{ | ||
"trailingComma": "es5", | ||
"tabWidth": 2, | ||
"semi": false, | ||
"singleQuote": true | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
import globals from "globals"; | ||
import pluginJs from "@eslint/js"; | ||
import tseslint from "typescript-eslint"; | ||
|
||
export default [ | ||
pluginJs.configs.recommended, | ||
...tseslint.configs.strict, | ||
// Todo: explore typed-linting: https://typescript-eslint.io/getting-started/typed-linting | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. More of a personal note. I'll remove this todo. |
||
{ | ||
languageOptions: { | ||
globals: globals.node, | ||
}, | ||
}, | ||
// global ignore | ||
{ | ||
ignores: ["node_modules/", "dist/"], | ||
}, | ||
{ | ||
rules: { | ||
"@typescript-eslint/no-unused-vars": "warn", | ||
"@typescript-eslint/no-empty-function": "warn", | ||
"no-empty": "warn", | ||
"no-constant-condition": "warn", | ||
}, | ||
}, | ||
]; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
{ | ||
"watch": [ | ||
"./src/**/*.ts" | ||
], | ||
"exec": "tsc || exit 1" | ||
} |
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 mostly made this to help me during development, but should I leave it around for users too?
The idea is that they run
wasp ts-setup
and Wasp turns a regular project into a TS SDK project.I could rename the command too, suggestions welcome.
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.
Hm can't say, what is their experience of testing out TS SDK right now with or without this command? If it doesn't improve their experience, maybe remove it. If it helps you a lot though, maybe keep it. Long term I am guessing we won't be having this command?
We now have 3 ts configs files instead of one. So how does that work -> when they run this command, it creates those 3 config files for them? I don't see them being created though in the code. So you tell them manually to create those files?
How do we imagine this in the future where we have Wasp DSL and TS SDK at the same time -> every project will come with at least two ts config files, and then the third one can be added if they want to use TS SDK? Or maybe every proejct comes with all three since the third one (for TS SDK) doesn't hurt?