Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Adds support for a client public folder #1229

Merged
merged 21 commits into from
Jun 15, 2023
Merged

Conversation

infomiho
Copy link
Contributor

@infomiho infomiho commented Jun 2, 2023

Fixes #9

If the user created a public folder in the their client folder -> it will get merged with the React app's public folder.

This means that users can expose client files at / more easily. It enables users to define a custom favicon.ico.

Left to do

  • Update the docs
  • Update the Changelog

infomiho added 2 commits June 2, 2023 11:11
Signed-off-by: Mihovil Ilakovac <mihovil@ilakovac.com>
Signed-off-by: Mihovil Ilakovac <mihovil@ilakovac.com>
Copy link
Member

@Martinsos Martinsos left a comment

Choose a reason for hiding this comment

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

Looking quite good and very nice job on the Haskell code, it is looking very solid! Also clean!

There are some intricacies to how we handle file copying though, Wasp currently, that I will make sure to explain the best, as they might push this PR in a bit different direction.

So we have two ways of producing files in the generated project -> via file drafts, or via other methods. Other methods are "normal" ways -> copying files directly basically, per case. We do that for migrations, I think for something else also.
And file drafts -> we use that for all the user source code (external code), and we also use it for any files that we come up with in the Generator itself, be it based on templates or directly from memory or whatever.

So basically code itself we generate via file drafts, and a couple of things around it we handle directly (like migrations).

The benefit of file drafts (fds from now on) is that we have a system for writing them to disk, where we ensure that we don't write them if it is not needed. We also make sure there are no old file drafts laying around, that could mess up the generated project. And that all happens automatically, as long as you use file draft mechanism, so this is a preffered way of generating code in the generated Wasp project.

As for static assets -> since they are in the client/ dir, where the rest of the external code is -> I am thinking that we should actually probably also use the file drafts method! We could handle them similarly as we handle external code files. Actually, if you put them right now into public/ dir in client/, so in client/public/, don't they already get copied over by themselves, due to us copying over everything from client/?
The code that turns ext code dir into fds is in Wasp.ExternalCode -> readFiles. And it is called in a couple of places, one of them calling it on the client/. So hm, I am actually thinking now that it should get copied automatically, I would be surprised if it doesn't.

@infomiho
Copy link
Contributor Author

infomiho commented Jun 2, 2023

Other methods are "normal" ways -> copying files directly basically, per case. We do that for migrations

We are doing createCopyDirFileDraft for the migrations folder (I've used that method for this as well)

don't they already get copied over by themselves, due to us copying over everything from client/?

Yes they do, but it's not good enough since it's copied in src/ext-src but it needs to be in public (same level as src) for Vite to pick it up.

Behaviour before this PR:
(user land) src/client/file.txt -> (wasp land) src/ext-src/file.txt -> (build the app for production) poof it's gone

Behaviour after:
(user land) src/client/public/file.txt -> (wasp land) public/file.txt -> (build the app for production) /file.txt

@Martinsos
Copy link
Member

Aha got it!
Yes we are doing it with migrations but they are a bit special, and we couldn't fit them in into the FD methodology.

Behaviour before this PR:
(user land) src/client/file.txt -> (wasp land) src/ext-src/file.txt -> (build the app for production) poof it's gone

Did you mean:
Behaviour before this PR:
(user land) src/client/public/file.txt -> (wasp land) src/ext-src/public/file.txt -> (build the app for production) poof it's gone

Can't we tell Vite to look into ext-src/public and that is it?

Have you then ensured they are not also copied as external code files -> I guess they are also copied like that now then?

One thing we could potentially do is edit External.Code.readFiles so it takes an additional argument, a function, that receives original path of the file, and returns destination path, allowing readFiles to reshuffle files a bit if it wants. And then we could provide a function in that places that is id for most of the files, but if it recognizes they are in public dir, it would adjust a path to put it a couple levels up.

@infomiho
Copy link
Contributor Author

infomiho commented Jun 2, 2023

Can't we tell Vite to look into ext-src/public and that is it?

It will work yes. But it feels off seeing src/ext-src/public as the public dir, but users won't see it, so it's our implementation detail.

Also, we already have a public folder and you can only specify one. Which would mean that our default folder should be src/ext-src/public which feels off to me personally -> even if the user doesn't put anything in their client/public

@Martinsos
Copy link
Member

Can't we tell Vite to look into ext-src/public and that is it?

It will work yes. But it feels off seeing src/ext-src/public as the public dir, but users won't see it, so it's our implementation detail.

Also, we already have a public folder and you can only specify one. Which would mean that our default folder should be src/ext-src/public which feels off to me personally -> even if the user doesn't put anything in their client/public

Yeah ok that is weird, since our stuff also goes in, that is true.

infomiho and others added 7 commits June 13, 2023 14:12
Co-authored-by: Martin Šošić <Martinsos@users.noreply.github.com>
Signed-off-by: Mihovil Ilakovac <mihovil@ilakovac.com>
Signed-off-by: Mihovil Ilakovac <mihovil@ilakovac.com>
Copy link
Member

@Martinsos Martinsos left a comment

Choose a reason for hiding this comment

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

@infomiho looking very good, I believe there is only one comment left open, the one with paths, prefixes and "/", check it out.

Signed-off-by: Mihovil Ilakovac <mihovil@ilakovac.com>
@@ -1872,6 +1872,25 @@ Make sure to pass in an object expected by the `QueryClient`'s constructor, as
explained in
[_react-query_'s docs](https://tanstack.com/query/v4/docs/react/reference/QueryClient).

## Public static files on the client
Copy link
Member

Choose a reason for hiding this comment

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

Can they import stuff from there if they want? What can they do with the stuff that ends up there? What can they put there besides favicon.ico?

Copy link
Member

@Martinsos Martinsos left a comment

Choose a reason for hiding this comment

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

Great! Left one tiny comment for docs, but this is it from me, I am all good. Looking nice!

infomiho added 4 commits June 15, 2023 13:06
Signed-off-by: Mihovil Ilakovac <mihovil@ilakovac.com>
Signed-off-by: Mihovil Ilakovac <mihovil@ilakovac.com>
@infomiho infomiho merged commit b37cc22 into main Jun 15, 2023
@infomiho infomiho deleted the miho-public-folder-support branch June 15, 2023 13:17
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add favicon support
2 participants