Using TS SDK from `waspc` #2276

sodic · 2024-09-11T14:30:38Z

These PR contains the changes that enable waspc to process TS SDK projects that look like this.

.
├── main.wasp.mts         // Wasp config file
├── package.json
├── package-lock.json
├── public
├── schema.prisma
├── src
├── tsconfig.src.json     // tsconfig for src
├── tsconfig.json         // Composite  tsconfig
├── tsconfig.wasp.json    // tsconfig for the Wasp config file
├── vite.config.ts
└── node_moduels
    ├── ...
    └── wasp-config       // The package for the TS sdk

The PR doesn't contain any code for creating the artifacts necessary for such a project (e.g., tsconfig templates, the wasp-config package), etc. It's just the Haskell code required to process it.

It's just the purple parts from this graph (the prep work and the blue parts are coming in #2299). The idea is to review this PR by treating the blue parts as a black box with a simple runNodeCommandAsJob interface (but if you really really want a project to try it out, I can hook you up).

Martinsos · 2024-09-26T10:32:58Z

waspc/src/Wasp/Project/Analyze.hs

sodic · 2024-09-26T09:44:41Z

waspc/src/Wasp/Psl/Ast/Schema.hs

+getModelNames :: Schema -> [String]
+getModelNames schema = map Model.getName $ getModels schema


@Martinsos I realize all other functions in the file are just "getters" for the Schema fields and that this breaks module's "purity," but I strongly believe this function should exist (it decouples the caller from the schema's internals).

Where should I move it, if anywhere? I like it here, but I seem to recall you asking me to stick with the getter-only rule in the past.

There's always Wasp.Psl.Util which seems like an okay place to put this helper.

Well it is not so terrible, but I get what you mean: I mean it is quite random, why can't caller just do Model.getName <$> getModels schema? I don't see much value in decoupling caller from schema's internals -> the fact that schema has models in it are not internals, nor that each model has a name, it is basically its API.
The fact that you have a caller which needs just model names but doesn't care how exactly to fetch models or a name from a model doesn't seem like a good enough reason to create a whole new API here for that use case.

So I would say, either move this function to wherever you need it (so much close to the caller and out of Wasp/Psl module subtree), or even better, get rid of this function and just inline it wherever you are using it -> again, I don't think you gain anyting by abstracting stuff here.

Model.getName <$> getModels schema

This is the part I'm not a fan of. We do that often (inline accesses to source-of-truth objects), but it always bugs me a little.

My main motivation here is Law of Demeter.

The counter-argument (if I got it correctly) is that, in this case, the schema's internal structure is so inherent that there's no point in hiding it (and that's why this particular instance is not the best example, but bear with me).

I generally dislike chaining multiple "accessor" calls. It requires importing multiple symbols, and refactoring many imports and call sites whenever I make a simple name change.

I'm talking about the stuff like:

import Foo import Bar import AppSpec ... Foo.getBar <$> Foo.getBars $ AS.getFoo spec -- then later again ... Foo.getBar <$> Foo.getBars $ AS.getFoo spec

Sometimes someone notices the repetition and defines a local helper function (like you suggest here). Then, in a different location, someone else notices the repetition again and defines an identical helper function (again local). We end up with N inline queries and M local functions that all do the same thing.

For example, this getModelNames function is used in two modules: Wasp.Project.Analyze and Wasp.LSP.Prisma.

Ideally, If I want something from the AppSpec in the generator, I don't want to import the entire hierarchy and inline the same expression multiple times.
I want to import only a single function that gives me what I want and then use that.

In simple terms, I'd prefer if our source-of-truth objects (like the AppSpec and the schema) had a higher-level API for stuff that's often used. It would be perfect if we could use an index file to reexport stuff we want to expose and "ban" all lower-level imports (but I don't think Haskell can do this).

That makes sense to me, but these accessors can then get quite specific. What I don't like then is that I am looking at lets say AppSpec code, and I see these couple of quite custom accessors, which are on higher level. But then, for other stuff in the AppSpec, there are no such higher level accessors. Which is good I guess, because there would have to be a bunch of them to have a complete "higher level api", but on the other hand, why did we pick these ones then? Answer is, because they were used a lot. So that means that for less used stuff, we are still uisng lower level API? So now we are deciding if API is public or not based on frequency of usage?

And yeah, what you said about inherency of the structure is important. Or I would say, inherency of the API. Or, I would say, how public is API. If API is public, I don't see an issue with it being somewhat lower level.

Sounds like you are solving decoupling problem above, but it is not really a problem because we intend that API to be used like that, there is no "implementation" leaking out, but you introduce the new problem of adding higher level API but only a bit of it which I think starts a mess.

The case when this makes sense, I think, is if

It is really implementation that is leaking.

There is opportunity to introduce complete higher level API, not pieces of it.

Otherwise, if we need only piece of that higher level API, I think it is indeed better to have it defined at the place of usage. If you don't like that one being redefined on multiple places, well you can make a common place where it is defined so those callers can share it -> it still doesn't have to be part of the AppSpec.

Btw, I might be ok with it if it was some more complex stuff. Some common operation that uses multiple lower level mechanisms. But then it would be ok, because it would probably not be just a piece of higher level API, it would be a complete higher level API for that thing. Here, when we have just the combining of accessors for public parts of the structure, there is so many combos that it doesn't make much sense to pick just a few combos and claim that you did a decoupling.

I'm leaving this one for later as well.

sodic · 2024-09-26T11:46:26Z

waspc/src/Wasp/Project/Analyze.hs

-      ".wasp"
-        `isSuffixOf` toFilePath path
-        && (length (toFilePath path) > length (".wasp" :: String))
+    findWaspTsFile files = WaspTs <$> findFileThatEndsWith ".wasp.mts" files


I explained the mjs and mts story in Discord, you can ignore that for now.

If that sticks, we will also wnat to have a comment in code somewhere that explains it.

waspc/test/AppSpec/FromJSONTest.hs

sodic · 2024-09-26T11:49:43Z

waspc/src/Wasp/Project/Analyze.hs

+          -- TODO: Figure out how to keep running instructions in a single
+          -- place (e.g., this is string the same as the package name, but it's
+          -- repeated in two places).
+          -- Before this, I had the entrypoint file hardcoded, which was bad
+          -- too: waspProjectDir </> [relfile|node_modules/wasp-config/dist/run.js|]


This is a topic for the next PR (the one that sets up the package): #2299

sodic · 2024-09-26T11:50:22Z

waspc/src/Wasp/Project/Analyze.hs

+  -- TODO: I'm not yet sure where tsconfig.node.json location should come from
+  -- because we also need that knowledge when generating a TS SDK project.


This is a topic for the next PR (the one that sets up the package): #2299

waspc/src/Wasp/Project/Analyze.hs

waspc/src/Wasp/Psl/Ast/Model.hs

infomiho · 2024-09-26T13:50:22Z

waspc/src/Wasp/Project/Analyze.hs

@@ -60,7 +75,7 @@ analyzeWaspProject waspDir options = do
        (Left prismaSchemaErrors, prismaSchemaWarnings) -> return (Left prismaSchemaErrors, prismaSchemaWarnings)
        -- NOTE: we are ignoring prismaSchemaWarnings if the schema was parsed successfully
        (Right prismaSchemaAst, _) ->
-          analyzeWaspFile prismaSchemaAst waspFilePath >>= \case
+          analyzeWaspFile waspDir prismaSchemaAst waspFilePath >>= \case


waspFilePath -> waspFile?

Hmm, I prefer keeping the Path. I think it's our general convention too.

waspc/src/Wasp/Project/Analyze.hs

infomiho · 2024-09-26T14:05:01Z

LGTM, I'm looking forwards to reviewing other bits and testing this locally!

I'm eager to hear if we can get some IDE support for users when writing entities.

Martinsos

Nice stuff @sodic I am super excited about this!

Biggest comments are:

We could probably do with more comments (function header comments likely) in this code as it is quite complex / has design desicions that are not obivous from the code.
I didn't really understand the reasoning behind the comment referring to tsc call, I commented more on that there.

And the rest is covered by all the specific comments.

waspc/src/Wasp/Project/Analyze.hs

Martinsos · 2024-09-27T13:12:15Z

waspc/src/Wasp/Error.hs

@@ -12,7 +13,7 @@ import qualified Wasp.Util.Terminal as T
 -- | Transforms compiler error (error with parse context) into an informative, pretty String that
 -- can be printed directly into the terminal. It uses terminal features like escape codes
 -- (colors, styling, ...).
-showCompilerErrorForTerminal :: (Path' Abs File', String) -> (String, Ctx) -> String
+showCompilerErrorForTerminal :: (Path' Abs (File f), String) -> (String, Ctx) -> String


We don't have something like data WaspFile, so that we could do Path' Abs (File WaspFile)?

Nice catch. We didn't before, but we'll have it now.

I'll take care of this later. It requires dealing with cyclic dependencies.

Martinsos · 2024-09-27T13:20:00Z

waspc/src/Wasp/Project/Analyze.hs

+import qualified Wasp.Generator.Job as J
+import Wasp.Generator.Job.IO (readJobMessagesAndPrintThemPrefixed)
+import Wasp.Generator.Job.Process (runNodeCommandAsJob)


If this is used here, then it sounds like it maybe shouldn't be in the Generator hm. What do you think? I didn't check the code though to see if there is anything Generator specific about it really or not.

You're right, nice catch. I didn't notice it was there.

And there's nothing generator-specific about it. I think it's just there because the generator used to be the only one running jobs. I'll move it somewhere more appropriate.

Leaving this one for later.

Martinsos · 2024-09-27T13:22:22Z

waspc/src/Wasp/Project/Analyze.hs

+import StrongPath.TH (relfile)
+import StrongPath.Types (File)


I see somebody went wild with accepting import suggestions from LS :D. You can just add those to StrongPath import above. I also wonder if you need separate qualified as SP line.

Heh.

I have had this item on my Haskell list for quite some time.

For some reason (and for some SP imports), LSP almost never suggests a top-level import from StrongPath. Whenever I look at LSP's import suggestions, this is my protocol:

Check if there is an Add to import option.

Check if there is a top-level import option (in this case, StrongPath).

Use a lower-level import option and fix the import manually later (sometimes it works, sometimes it doesn't).

This mental overhead drives me crazy, and sometimes I forget to change the imports manually later (like here). I would really like to solve this because I don't want to waste time manually editing the imports.

@Martinsos @infomiho Is this specific to my setup, does it happen to you?
Please try with reldirP in Wasp.Project.Common. I just reproduced it there:

Top level for me

I am in emacs, so it is a bit different, but first I get a suggestion for autocompletion:

This one is correct, but when I accept, for some reason it doesn't also add an import automatically. So I get a compiler error that symbol is missing. I then tell LSP to suggest code actions, and then I get this

Which is correct! I am offered also the lower level imports, but the first option I am offered is to add it to existing import list of StrongPath, and if I pick that, it adds import correctly.

Ok, glad to hear it's fixable because it's been annoying me for ages (particularly with StrongPath imports). I'll bother you in the office to help me figure it out.

waspc/src/Wasp/Project/Analyze.hs

Martinsos · 2024-09-27T16:31:59Z

waspc/src/Wasp/Project/Analyze.hs

-      ".wasp"
-        `isSuffixOf` toFilePath path
-        && (length (toFilePath path) > length (".wasp" :: String))
+    findWaspTsFile files = WaspTs <$> findFileThatEndsWith ".wasp.mts" files


If that sticks, we will also wnat to have a comment in code somewhere that explains it.

waspc/src/Wasp/Project/Common.hs

Martinsos · 2024-09-27T16:40:03Z

waspc/src/Wasp/Psl/Ast/Schema.hs

+getModelNames :: Schema -> [String]
+getModelNames schema = map Model.getName $ getModels schema


Well it is not so terrible, but I get what you mean: I mean it is quite random, why can't caller just do Model.getName <$> getModels schema? I don't see much value in decoupling caller from schema's internals -> the fact that schema has models in it are not internals, nor that each model has a name, it is basically its API.
The fact that you have a caller which needs just model names but doesn't care how exactly to fetch models or a name from a model doesn't seem like a good enough reason to create a whole new API here for that use case.

So I would say, either move this function to wherever you need it (so much close to the caller and out of Wasp/Psl module subtree), or even better, get rid of this function and just inline it wherever you are using it -> again, I don't think you gain anyting by abstracting stuff here.

waspc/waspls/src/Wasp/LSP/Prisma/Util.hs

sodic · 2024-10-07T20:17:17Z

waspc/src/Wasp/Analyzer.hs

+getEntityDecls :: Psl.Schema.Schema -> Either [AnalyzeError] [Decl]
+getEntityDecls schema =
+  wrapAnalyzerError TypeError (typeCheck stdTypes astWithEntitiesOnly)
+    >>= (wrapAnalyzerError EvaluationError . evaluate stdTypes)
+  where
+    astWithEntitiesOnly = Parser.AST $ parseEntityStatements schema


@Martinsos This is what we discussed on Discord: sending only the entity statements through the compilation pipeline to get their declarations.

Should I document the idea behind it?

sodic · 2024-10-07T20:17:37Z

waspc/src/Wasp/Analyzer/Evaluator/Evaluation/TypedExpr/Combinators.hs

+    case AppSpec.ExtImport.parseExtImportPath extImportPath of
+      Left err -> mkParseError ctx err
+      Right importPath -> pure $ AppSpec.ExtImport.ExtImport name importPath


I needed this for FromJSON so I moved it.

sodic · 2024-10-07T20:19:28Z

waspc/src/Wasp/AppSpec/ExtImport.hs

+    extImportPath <- case parseExtImportPath pathStr of
+      Right path' -> pure path'
+      Left err -> fail err


@Martinsos Any helper function for this? I can't use left because of fail.

sodic · 2024-10-07T20:41:29Z

waspc/src/Wasp/Project/Common.hs

 tsConfigInWaspProjectDir :: Path' (Rel WaspProjectDir) (File TsConfigFile)
-tsConfigInWaspProjectDir = [relfile|tsconfig.json|]
+tsConfigInWaspProjectDir = [relfile|tsconfig.app.json|]


This is a problem, fix it.

sodic · 2024-10-08T09:56:15Z

waspc/src/Wasp/Analyzer/Evaluator/Evaluation/TypedExpr/Combinators.hs

@@ -156,33 +156,18 @@ tuple4 eval1 eval2 eval3 eval4 = evaluation $ \(typeDefs, bindings) -> withCtx $
 extImport :: TypedExprEvaluation AppSpec.ExtImport.ExtImport
 extImport = evaluation' . withCtx $ \ctx -> \case
  TypedAST.ExtImport name extImportPath ->
+    -- NOTE(martin): This parsing here could instead be done in Parser.


Remove this

sodic added 20 commits September 6, 2024 18:23

Successfully get spec from stdout

e228263

Successfully read spec from file

664eafe

Clean up code in Analyze.hs

1b75ab9

Move outDir source of truth into Haskell

0c7b901

Add extra context to Analyze.hs

b7c4800

Improve TS SDK analysis

68bfe30

Put config dir back in .wasp dir

0758747

Add more comments

b7ecab4

Update Analyze.hs

e2e802e

Merge branch 'main' into filip-ts-sdk

2d27900

Remove todo for duplication

987243d

Remove the duplication in templating

15e3e68

Rearrange analyze.hs

7600288

Remove todo after adding it to GitHub

2b0c0ff

Remove one more todo

7ec9356

Remove todo after adding it to GitHub

1597f4b

Resolve some more todos

b887cd8

Remove another todo

1bdc871

Send prisma models to JS runtime

52378c4

Merge branch 'main' into filip-ts-sdk

115dce5

sodic added 2 commits September 26, 2024 13:41

Properly parse decls.json

2617b6f

Improve error messages

32275e5

sodic changed the title ~~TS Sdk~~ Using TS SDK from waspc Sep 26, 2024

sodic commented Sep 26, 2024

View reviewed changes

sodic marked this pull request as ready for review September 26, 2024 12:12

sodic commented Sep 26, 2024

View reviewed changes

waspc/src/Wasp/Project/Analyze.hs Outdated Show resolved Hide resolved

sodic requested review from Martinsos and infomiho September 26, 2024 12:30

infomiho reviewed Sep 26, 2024

View reviewed changes

waspc/src/Wasp/Psl/Ast/Model.hs Show resolved Hide resolved

infomiho reviewed Sep 26, 2024

View reviewed changes

waspc/src/Wasp/Project/Analyze.hs Outdated Show resolved Hide resolved

Martinsos reviewed Sep 27, 2024

View reviewed changes

sodic added 2 commits September 30, 2024 11:41

Merge branch 'main' into filip-ts-sdk

b673308

Fix ext import parsing and inject entities

3513b68

This was referenced Oct 7, 2024

TS SDK Setup #2299

Open

Figure out the preconditions behind runNodeCommandAsJob #2324

Open

sodic added 11 commits October 7, 2024 20:39

Fail if you find both wasp files

e9ef283

Change tsconfig file name

0dc7435

Clean up imports

3e9efc8

Fix some more imports

24f65ec

Change type name

edd7a85

Implement review changes

ae76820

Fix failing tests

7eab0bc

Remove duplication when parsing ext imports

8c8cb4f

Remove redundant variable

d0ba9e5

Remove trailling space

4eec3c1

Revert Show instance for Decl

1699df2

sodic commented Oct 8, 2024

View reviewed changes

sodic added 2 commits October 8, 2024 13:53

Add module to package.json and remove mts

5a6719e

Merge branch 'main' into filip-ts-sdk

3f48b98

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Using TS SDK from `waspc` #2276

Using TS SDK from `waspc` #2276

sodic commented Sep 11, 2024 •

edited

Loading

Martinsos commented Sep 26, 2024

sodic Sep 26, 2024

infomiho Sep 26, 2024

Martinsos Sep 27, 2024

sodic Sep 30, 2024

Martinsos Oct 1, 2024 •

edited

Loading

sodic Oct 7, 2024

sodic Sep 26, 2024

Martinsos Sep 27, 2024

sodic Sep 26, 2024 •

edited

Loading

sodic Sep 26, 2024 •

edited

Loading

infomiho Sep 26, 2024

sodic Oct 7, 2024

infomiho commented Sep 26, 2024

Martinsos left a comment

Martinsos Sep 27, 2024

sodic Sep 30, 2024

sodic Oct 7, 2024

Martinsos Sep 27, 2024

sodic Sep 30, 2024

sodic Oct 7, 2024

Martinsos Sep 27, 2024

sodic Sep 30, 2024

infomiho Sep 30, 2024

Martinsos Oct 1, 2024

sodic Oct 7, 2024

Martinsos Sep 27, 2024

Martinsos Sep 27, 2024

sodic Oct 7, 2024

sodic Oct 7, 2024

sodic Oct 7, 2024

sodic Oct 7, 2024

sodic Oct 8, 2024

		getModelNames :: Schema -> [String]
		getModelNames schema = map Model.getName $ getModels schema

		-- TODO: I'm not yet sure where tsconfig.node.json location should come from
		-- because we also need that knowledge when generating a TS SDK project.

		import StrongPath.TH (relfile)
		import StrongPath.Types (File)

Using TS SDK from waspc #2276

Are you sure you want to change the base?

Using TS SDK from waspc #2276

Conversation

sodic commented Sep 11, 2024 • edited Loading

Martinsos commented Sep 26, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Martinsos Oct 1, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sodic Sep 26, 2024 • edited Loading

Choose a reason for hiding this comment

sodic Sep 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

infomiho commented Sep 26, 2024

Martinsos left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Using TS SDK from `waspc` #2276

Using TS SDK from `waspc` #2276

sodic commented Sep 11, 2024 •

edited

Loading

Martinsos Oct 1, 2024 •

edited

Loading

sodic Sep 26, 2024 •

edited

Loading

sodic Sep 26, 2024 •

edited

Loading