Skip to content
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.

Sign up for GitHub

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

Validates tailwindcss if Tailwind is used #2465

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Validates tailwindcss if Tailwind is used #2465

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1af064b
Validates tailwindcss if Tailwind is used
infomiho Jan 23, 2025
1df40ff
e2e tests
infomiho Jan 23, 2025
f1a8075
Fixes todoApp
infomiho Jan 23, 2025
b6fae16
Update headless tests
infomiho Jan 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion waspc/e2e-test/test-outputs/waspBuild-golden/waspBuild/main.wasp
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion waspc/e2e-test/test-outputs/waspCompile-golden/waspCompile/main.wasp
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion waspc/e2e-test/test-outputs/waspComplexTest-golden/waspComplexTest/main.wasp
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion waspc/e2e-test/test-outputs/waspJob-golden/waspJob/main.wasp
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion waspc/e2e-test/test-outputs/waspMigrate-golden/waspMigrate/main.wasp
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion waspc/e2e-test/test-outputs/waspNew-golden/waspNew/main.wasp
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion waspc/examples/todoApp/main.wasp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
app todoApp {
wasp: {
version: "^0.15.0"
version: "^0.16.0"
},
title: "ToDo App",
// head: [],
Expand Down
1 change: 1 addition & 0 deletions waspc/examples/todoApp/package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion waspc/examples/todoApp/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@
"react": "^18.2.0",
"react-dom": "^18.2.0",
"react-router-dom": "^6.26.2",
"wasp": "file:.wasp/out/sdk/wasp"
"wasp": "file:.wasp/out/sdk/wasp",
"tailwindcss": "^3.2.7"
},
"devDependencies": {
"@types/express": "^4.17.13",
Expand Down
7 changes: 1 addition & 6 deletions waspc/src/Wasp/AppSpec.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ module Wasp.AppSpec
getRoutes,
getJobs,
resolveRef,
doesConfigFileExist,
asAbsWaspProjectDirFile,
getApp,
getApiNamespaces,
Expand All @@ -26,7 +25,7 @@ module Wasp.AppSpec
where

import Data.List (find)
import Data.Maybe (fromMaybe, isJust)
import Data.Maybe (fromMaybe)
import Data.Text (Text)
import StrongPath (Abs, Dir, File', Path', Rel, (</>))
import Wasp.AppSpec.Action (Action)
Expand Down Expand Up @@ -149,10 +148,6 @@ resolveRef spec ref =
$ find ((== refName ref) . fst) $
getDecls spec

doesConfigFileExist :: AppSpec -> Path' (Rel WaspProjectDir) File' -> Bool
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I needed the information if Tailwind is used (which is computed using doesConfigFileExist) before we have the AppSpec so I pulled it out of here and rewritten it so it doesn't need the AppSpec.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is it that you needed it before?
Are we getting into a situtation where we need a lot of info from AppSpec before AppSpec exists, pointing at some potential design issue, or do you think this is ok?

doesConfigFileExist spec file =
isJust $ find ((==) file . _pathInWaspProjectDir) (configFiles spec)

asAbsWaspProjectDirFile :: AppSpec -> Path' (Rel WaspProjectDir) File' -> Path' Abs File'
asAbsWaspProjectDirFile spec file = waspProjectDir spec </> file

Expand Down
18 changes: 12 additions & 6 deletions waspc/src/Wasp/Generator/ConfigFile.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,11 @@ module Wasp.Generator.ConfigFile
)
where

import Data.List (find)
import Data.Map (fromList)
import Data.Maybe (isJust)
import StrongPath (File', Path', Rel, castRel, relfile, (</>))
import Wasp.AppSpec (AppSpec, doesConfigFileExist)
import qualified Wasp.AppSpec.ConfigFile as CF
import Wasp.ConfigFile (ConfigFileRelocationMap)
import Wasp.Generator.Common (ProjectRootDir)
import Wasp.Generator.WebAppGenerator.Common (webAppRootDirInProjectRootDir)
Expand All @@ -24,12 +26,16 @@ asProjectRootDirConfigFile = (webAppRootDirInProjectRootDir </>) . castRel
-- | Helper that determines if Tailwind used. For our purposes, we allow
-- developers to opt-in by creating both a tailwind config file and
-- postcss config file in their wasp project dir.
isTailwindUsed :: AppSpec -> Bool
isTailwindUsed spec =
doesConfigFileExist spec tailwindConfigFile
&& doesConfigFileExist spec postcssConfigFile
isTailwindUsed :: [CF.ConfigFileRelocator] -> Bool
isTailwindUsed configFileRelocators =
doesConfigFileExist tailwindConfigFile
&& doesConfigFileExist postcssConfigFile
where
doesConfigFileExist :: Path' (Rel WaspProjectDir) File' -> Bool
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is only used for isTailwindUsed so I inlined it even though it might be useful in the future - let's pull it out when it becomes necessary?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you know it is a general utility, I would pull it out immediately, since then it is easier for somebody else to see it there and use it. If it was more tied down to this specific place then mabye not, but it seems quite ready to isolate right now, and I can easily imagine somebody else will want to check if certain congfi file exists, so yeah, let's go for it right now.

doesConfigFileExist file =
isJust $ find ((==) file . CF._pathInWaspProjectDir) configFileRelocators

-- | Establishes the mapping of what config files to copy and where from/to.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ormolu

-- Establishes the mapping of what config files to copy and where from/to.
-- NOTE: In the future, we could allow devs to configure what files we look for and where we copy them.
configFileRelocationMap :: ConfigFileRelocationMap
configFileRelocationMap =
Expand Down
27 changes: 17 additions & 10 deletions waspc/src/Wasp/Generator/ExternalConfig/PackageJson.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,22 +7,29 @@ import qualified Data.Map as M
import qualified Wasp.ExternalConfig.PackageJson as P
import Wasp.Generator.Common (prismaVersion)
import Wasp.Generator.ExternalConfig.Common (ErrorMsg)
import Wasp.Generator.SdkGenerator.Common (tailwindCssVersion)
import Wasp.Generator.WebAppGenerator.Common (reactRouterVersion, reactVersion)

validatePackageJson :: P.PackageJson -> [ErrorMsg]
validatePackageJson packageJson =
validatePackageJson :: Bool -> P.PackageJson -> [ErrorMsg]
validatePackageJson isTailwindUsed packageJson =
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to thread through the isTailwindUsed info unfortunately. Some sort of Reader would be useful here, but I didn't want to over-engineer it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get what you mean, and I agree it would be much going for Reader now.
That said, having just a Bool like this is not very nice, what if we add one or two more conditionals like this, would we have 2 more arguments? I would immediately go for grouping this into a record, even it if has just one field for now. Might be called something like ValidationConfig or ValidationPreconditions or ValidationContext (I like that one!) or whatever you think is the best name. That will also make moving to Reader easier in the future if we want it, since you would just move that record into Reader. But it will also make this function immediately more readable and extendable.

concat
[ validate ("wasp", "file:.wasp/out/sdk/wasp") IsListedWithExactVersion,
validate ("prisma", show prismaVersion) IsListedAsDevWithExactVersion,
-- Installing the wrong version of "react-router-dom" can make users believe that they
-- can use features that are not available in the version that Wasp supports.
validate ("react-router-dom", show reactRouterVersion) IsListedWithExactVersion,
validate ("react", show reactVersion) IsListedWithExactVersion,
validate ("react-dom", show reactVersion) IsListedWithExactVersion
]
( [ validate ("wasp", "file:.wasp/out/sdk/wasp") IsListedWithExactVersion,
validate ("prisma", show prismaVersion) IsListedAsDevWithExactVersion,
-- Installing the wrong version of "react-router-dom" can make users believe that they
-- can use features that are not available in the version that Wasp supports.
validate ("react-router-dom", show reactRouterVersion) IsListedWithExactVersion,
validate ("react", show reactVersion) IsListedWithExactVersion,
validate ("react-dom", show reactVersion) IsListedWithExactVersion
]
++ tailwindValidations
)
where
validate = validateDep packageJson

tailwindValidations =
[ validate ("tailwindcss", show tailwindCssVersion) IsListedWithExactVersion | isTailwindUsed
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If Tailwind is used, validate that the tailwindcss package is present in the package.json with the correct version.

]

data PackageValidationType = IsListedWithExactVersion | IsListedAsDevWithExactVersion | HasExactVersionIfListed

validateDep :: P.PackageJson -> (P.PackageName, P.PackageVersion) -> PackageValidationType -> [String]
Expand Down
5 changes: 3 additions & 2 deletions waspc/src/Wasp/Generator/SdkGenerator.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ import Wasp.Generator.SdkGenerator.Client.AuthG (genNewClientAuth)
import Wasp.Generator.SdkGenerator.Client.CrudG (genNewClientCrudApi)
import qualified Wasp.Generator.SdkGenerator.Client.OperationsGenerator as ClientOpsGen
import Wasp.Generator.SdkGenerator.Client.RouterGenerator (genNewClientRouterApi)
import Wasp.Generator.SdkGenerator.Common (tailwindCssVersion)
import qualified Wasp.Generator.SdkGenerator.Common as C
import Wasp.Generator.SdkGenerator.CrudG (genCrud)
import Wasp.Generator.SdkGenerator.EnvValidation (depsRequiredByEnvValidation, genEnvValidation)
Expand Down Expand Up @@ -279,10 +280,10 @@ depsRequiredForAuth spec = maybe [] (const authDeps) maybeAuth

depsRequiredByTailwind :: AppSpec -> [AS.Dependency.Dependency]
depsRequiredByTailwind spec =
if G.CF.isTailwindUsed spec
if G.CF.isTailwindUsed $ AS.configFiles spec
then
AS.Dependency.fromList
[ ("tailwindcss", "^3.2.7"),
[ ("tailwindcss", show tailwindCssVersion),
("postcss", "^8.4.21"),
("autoprefixer", "^10.4.13")
]
Expand Down
4 changes: 4 additions & 0 deletions waspc/src/Wasp/Generator/SdkGenerator/Common.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import Wasp.Generator.ExternalCodeGenerator.Common (GeneratedExternalCodeDir)
import Wasp.Generator.FileDraft (FileDraft, createTemplateFileDraft)
import Wasp.Generator.Templates (TemplatesDir)
import Wasp.Project.Common (generatedCodeDirInDotWaspDir)
import qualified Wasp.SemanticVersion as SV
import Wasp.Util (toUpperFirst)

data SdkRootDir
Expand Down Expand Up @@ -84,3 +85,6 @@ serverTemplatesDirInSdkTemplatesDir = [reldir|server|]

getOperationTypeName :: AS.Operation.Operation -> String
getOperationTypeName operation = toUpperFirst (AS.Operation.getName operation) ++ "_ext"

tailwindCssVersion :: SV.ComparatorSet
tailwindCssVersion = SV.backwardsCompatibleWith $ SV.Version 3 2 7
13 changes: 9 additions & 4 deletions waspc/src/Wasp/Project/Analyze.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import StrongPath
fromAbsDir,
)
import qualified Wasp.AppSpec as AS
import qualified Wasp.AppSpec.ConfigFile as CF
import Wasp.AppSpec.Core.Decl.JSON ()
import qualified Wasp.AppSpec.Valid as ASV
import Wasp.CompileOptions (CompileOptions)
Expand Down Expand Up @@ -51,6 +52,10 @@ analyzeWaspProject ::
IO (Either [CompileError] AS.AppSpec, [CompileWarning])
analyzeWaspProject waspDir options = do
waspFilePathOrError <- left (: []) <$> findWaspFile waspDir
configFiles <- CF.discoverConfigFiles waspDir G.CF.configFileRelocationMap

let isTailwindUsed = G.CF.isTailwindUsed configFiles

case waspFilePathOrError of
Left err -> return (Left err, [])
Right waspFilePath ->
Expand All @@ -61,25 +66,25 @@ analyzeWaspProject waspDir options = do
analyzeWaspFile waspDir prismaSchemaAst waspFilePath >>= \case
Left errors -> return (Left errors, [])
Right declarations ->
EC.analyzeExternalConfigs waspDir (getSrcTsConfigInWaspProjectDir waspFilePath) >>= \case
EC.analyzeExternalConfigs isTailwindUsed waspDir (getSrcTsConfigInWaspProjectDir waspFilePath) >>= \case
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't sure if I wanted to thread the configFiles all the way down to validatePackageJson and the call the G.CF.isTailwindUsed check. Since validatePackageJson doesn't need the list of configs, I decided not to.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm maybe they don't need it all, but this function is called analyzeExternalConfigs and now it takes a bool, waspDir, and then a specific config file (srcTsConfig). That is now quite messy, because you could look at those args as:
arg#1: info about config file
arg#2: context
arg#3: config file

Usually in Haskell we aim for "context" args at start, and then "core" args after that.
Since this function is about config files, any amount of them, then I would also find it more natural that it takes a list or record of config files, note one per argument.

So what I would consider is turning this function's signature into :: WaspDir -> ConfigFiles -> ReturnType (I used a bit pseudo names here).

And it's not a problem if it takes a bit too much info -> it will analyze what it needs to analyze. We don't want to guess how it will analyze them, that is implementation detail.

Left errors -> return (Left errors, [])
Right externalConfigs -> constructAppSpec waspDir options externalConfigs prismaSchemaAst declarations
Right externalConfigs -> constructAppSpec waspDir options externalConfigs configFiles prismaSchemaAst declarations

constructAppSpec ::
Path' Abs (Dir WaspProjectDir) ->
CompileOptions ->
EC.ExternalConfigs ->
[CF.ConfigFileRelocator] ->
Psl.Schema.Schema ->
[AS.Decl] ->
IO (Either [CompileError] AS.AppSpec, [CompileWarning])
constructAppSpec waspDir options externalConfigs parsedPrismaSchema decls = do
constructAppSpec waspDir options externalConfigs configFiles parsedPrismaSchema decls = do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What confuses me a bit is that this function takes both external configs and congif file relocators. Actually those two kind of make sense -> I guess it takes them, and then rules how to relocate them, I am not up to date with this piece of code completely so I am not quite sure -> but then for relocators you use configFiles name and that confuses me absolutely, because you said relocators but now it is files.

externalCodeFiles <- ExternalFiles.readCodeFiles waspDir
externalPublicFiles <- ExternalFiles.readPublicFiles waspDir
customViteConfigPath <- findCustomViteConfigPath waspDir

maybeMigrationsDir <- findMigrationsDir waspDir
maybeUserDockerfileContents <- loadUserDockerfileContents waspDir
configFiles <- CF.discoverConfigFiles waspDir G.CF.configFileRelocationMap
let dbSystem = getValidDbSystemFromPrismaSchema parsedPrismaSchema
let devDbUrl = makeDevDatabaseUrl waspDir dbSystem decls
serverEnvVars <- readDotEnvServer waspDir
Expand Down
5 changes: 3 additions & 2 deletions waspc/src/Wasp/Project/ExternalConfig.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,12 @@ data ExternalConfigs = ExternalConfigs
deriving (Show)

analyzeExternalConfigs ::
Bool ->
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general, seeing a Bool like this, where I can't tell from signature at all what it could be, is a bit suspicious. I commented more above why this one is probably not the right choice here.

Path' Abs (Dir WaspProjectDir) ->
Path' (Rel WaspProjectDir) (File SrcTsConfigFile) ->
IO (Either [CompileError] ExternalConfigs)
analyzeExternalConfigs waspDir srcTsConfigFile = runExceptT $ do
packageJsonContent <- ExceptT $ analyzePackageJsonFile waspDir
analyzeExternalConfigs isTailwindUsed waspDir srcTsConfigFile = runExceptT $ do
packageJsonContent <- ExceptT $ analyzePackageJsonFile isTailwindUsed waspDir
tsConfigContent <- ExceptT $ analyzeSrcTsConfigFile waspDir srcTsConfigFile

return $
Expand Down
6 changes: 3 additions & 3 deletions waspc/src/Wasp/Project/ExternalConfig/PackageJson.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@ import Wasp.Project.Common
)
import qualified Wasp.Util.IO as IOUtil

analyzePackageJsonFile :: Path' Abs (Dir WaspProjectDir) -> IO (Either [String] P.PackageJson)
analyzePackageJsonFile waspProjectDir = runExceptT $ do
analyzePackageJsonFile :: Bool -> Path' Abs (Dir WaspProjectDir) -> IO (Either [String] P.PackageJson)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably rework this signature also, with similar approach as to how I suggested in other comments.

analyzePackageJsonFile isTailwindUsed waspProjectDir = runExceptT $ do
packageJsonFile <- ExceptT findPackageJsonFileOrError
packageJson <- ExceptT $ readPackageJsonFile packageJsonFile
case validatePackageJson packageJson of
case validatePackageJson isTailwindUsed packageJson of
[] -> return packageJson
errors -> throwError errors
where
Expand Down
1 change: 0 additions & 1 deletion waspc/waspc.cabal
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,6 @@ library
Wasp.Generator.NpmInstall
Wasp.Generator.NpmInstall.Common
Wasp.Generator.NpmInstall.InstalledNpmDepsLog
Wasp.Generator.ImportPathAlias
Wasp.Generator.SdkGenerator
Wasp.Generator.SdkGenerator.Auth.AuthFormsG
Wasp.Generator.SdkGenerator.Auth.EmailAuthG
Expand Down
Loading