Adds support for root component #1009

infomiho · 2023-02-16T08:41:35Z

Root component

Fixes #77

This PR introduces rootComponent in the app.client dictionary in Wasp lang which will help users to define their root React component.

rootComponent is of type ExtImport which is an import statement from user defined files.

This enables the user to do:

Define a src/client/App.jsx which exports a component
Use the component to setup various providers e.g. Redux or ChakraProvider https://chakra-ui.com/getting-started

We then use their root component to wrap the router in the app's index.js.

Martinsos · 2023-02-16T09:31:29Z

waspc/data/Generator/templates/react-app/src/index.js

+      {=# rootComponent.isDefined =}
+      <{= rootComponent.importIdentifier =}>{router}</{= rootComponent.importIdentifier =}>
+      {=/ rootComponent.isDefined =}
+      {=^ rootComponent.isDefined =}
+      {router}
+      {=/ rootComponent.isDefined =}


Nitpick: What about:

Suggested change

{=# rootComponent.isDefined =}

<{= rootComponent.importIdentifier =}>{router}</{= rootComponent.importIdentifier =}>

{=/ rootComponent.isDefined =}

{=^ rootComponent.isDefined =}

{router}

{=/ rootComponent.isDefined =}

{=# rootComponent.isDefined =}

<{= rootComponent.importIdentifier =}>

{=/ rootComponent.isDefined =}

{router}

{=# rootComponent.isDefined =}

</{= rootComponent.importIdentifier =}>

{=/ rootComponent.isDefined =}

is this maybe nicer / more composable?

It will be easier to add new user defined wrappers, but will we have the need to add them in the future?

Your approach has extra whitespace in the output when not using rootComponent e.g. {router} 😂

Yeah I know but I gave up on formatting, we can't do it 100% anyway.

I think what I wrote is more explicit about "we are just wrapping it into this".

Makes sense, I'll probably need to give up on output formatting as well 😢

Maybe in the future, one of our build steps can be running everything through Prettier auto-format and then the users will get super nice output :D

yes that is probably what we will want to do! Or we will give up on it completely. But I like the idea of prettier.

Updated here: 207806c

Martinsos · 2023-02-16T09:35:08Z

Nice job -> your first extension of Wasp language itself :D!

Oh one thing is missing though -> docs, right? Let's add them in this PR, no reason to do them in separate PR.

Martinsos

Let's add docs?

infomiho · 2023-02-16T09:36:58Z

I've updated the docs in the meantime, check the last commit 🙂

Martinsos

LGTM! Some small comments, resolve as you find fit and you can merge!

Martinsos · 2023-02-16T09:39:49Z

web/docs/language/features.md

+export default async function Root({ children }) {
+  return (
+    <Provider store={store}>
+      <App>{children}</App>
+    </Provider>
+  )
+}
+
+function App({ children }) {
+  return (
+    <div>
+      <h1>My App</h1>
+      {children}
+    </div>
+  )
+}


WHy do we need this extra App here? Can't we just have Root render children directly in Provider component and that is it?

I wanted to offer a more elaborate example where we have the Provider wrapping and some <App /> compnent (which is how you'll see it's done in the Redux docs).

This also works:

export default async function Root({ children }) { return ( <Provider store={store}> {children} </Provider> ) }

Ok! It is good to give exapmles, but I sometimes have the need to reduce them if possible, to make maintainance simpler. But this is probably ok it is not so complex. Unless this might confused them -> do I have to use this App inside this thing? I don't know, your call!

I'll provide two use-case examples (Layout + Providers) and a one more complex where both are coming together.

Updated here: 207806c

waspc/src/Wasp/Generator/WebAppGenerator.hs

web/docs/language/features.md

sodic · 2023-02-16T14:39:16Z

waspc/src/Wasp/Generator/WebAppGenerator.hs

+mkTmplDataForExtImport :: Maybe ExtImport -> Aeson.Value
+mkTmplDataForExtImport maybeExtImport = maybe notDefinedValue mkTmplData maybeExtImport


This function encapsulates a common pattern.

If I understand correctly, we're using it for all ext imports. Do you think it makes sense to put it in a more public place and reuse?

This can be extracted to a common function in Wasp.Generator.JsImport and then it can be specialized in Wasp.Generator.WebAppGenerator.JsImport and Wasp.Generator.ServerGenerator.JsImport.

In the web app templates, this pattern is actually only used in this file, but in the server templates it's used more.

I'll extract the pattern in this PR and apply it to WebApp only. I'll do the same for server templates in a separate PR.

We now have:

Wasp.Generator.JsImport -> jsImportToTmplData which generates appropriate JSON for usage in templates when we need to do something with import statements

Wasp.Generator.WebAppGenerator.JsImport -> extImportToJsImportTmplData which uses the above method to convert an ExtImport to JSON template data

We should add in a separate PR:

Wasp.Generator.ServerGenerator.JsImport -> extImportToJsImportTmplData

use the new function to refactor the way we generate template data for JS import statements

To keep in line with naming conventions for functions (i.e., they should be verbs) and what we already have (like this guy here*), I suggest renaming this to something like makeImportJson.

The "functions should be verbs" applies to other places as well.

* Although that name could have been better as well :)

We have a number of these "tranform" functions in our codebase, and I kinda like this convetion of <A>To<B> naming.

Some examples I found in our codebase:

kebabToCamelCase

sourceSpanToRegion

lspPositionToOffset

extImportToJsImport (that's mine)

What about extImportToImportJson?

That's true, we have several of those.

Anyway, choose whatever you like best.

Purely FYI, this is my order of preference:

makeExtImportJson

extImportToImportJson

extImportToJsImport

I'm happy to inform you that I went with your second favourite option 🎉

Adds support for root component

38b8576

infomiho requested review from sodic and Martinsos and removed request for sodic February 16, 2023 08:41

infomiho added 2 commits February 16, 2023 10:15

Merge branch 'main' into miho-root-component-feature

9e79a14

Updates docs

790bd33

infomiho force-pushed the miho-root-component-feature branch from d9e6693 to 790bd33 Compare February 16, 2023 09:15

Martinsos approved these changes Feb 16, 2023

View reviewed changes

Martinsos self-requested a review February 16, 2023 09:35

Martinsos requested changes Feb 16, 2023

View reviewed changes

Martinsos approved these changes Feb 16, 2023

View reviewed changes

infomiho added 2 commits February 16, 2023 10:50

Merge branch 'main' into miho-root-component-feature

5408294

Updates docs and index.js template

207806c

sodic reviewed Feb 16, 2023

View reviewed changes

Updates JsImport usage a bit. Docs update.

cd83914

infomiho requested review from Martinsos and sodic February 16, 2023 15:29

infomiho added 4 commits February 17, 2023 14:22

Fixes relPathToWebAppSrcDir signature

a3fe220

Updates import locations signature. Updates naming

d21f1fa

Updates changelog

6eaf3e4

Update ChangeLog.md

4115f4a

infomiho merged commit c2a3e04 into main Feb 20, 2023

infomiho deleted the miho-root-component-feature branch February 20, 2023 10:15

infomiho mentioned this pull request Feb 28, 2023

Let devs define custom 404 page in Wasp #266

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds support for root component #1009

Adds support for root component #1009

infomiho commented Feb 16, 2023

Martinsos Feb 16, 2023

infomiho Feb 16, 2023

Martinsos Feb 16, 2023

infomiho Feb 16, 2023

Martinsos Feb 16, 2023

infomiho Feb 16, 2023

Martinsos commented Feb 16, 2023

Martinsos left a comment

infomiho commented Feb 16, 2023

Martinsos left a comment

Martinsos Feb 16, 2023

infomiho Feb 16, 2023 •

edited

Loading

Martinsos Feb 16, 2023

infomiho Feb 16, 2023 •

edited

Loading

infomiho Feb 16, 2023

sodic Feb 16, 2023

infomiho Feb 16, 2023

infomiho Feb 16, 2023 •

edited

Loading

sodic Feb 17, 2023 •

edited

Loading

infomiho Feb 17, 2023

sodic Feb 17, 2023

infomiho Feb 17, 2023

		mkTmplDataForExtImport :: Maybe ExtImport -> Aeson.Value
		mkTmplDataForExtImport maybeExtImport = maybe notDefinedValue mkTmplData maybeExtImport

Adds support for root component #1009

Adds support for root component #1009

Conversation

infomiho commented Feb 16, 2023

Root component

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

Martinsos commented Feb 16, 2023

Martinsos left a comment

Choose a reason for hiding this comment

infomiho commented Feb 16, 2023

Martinsos left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

infomiho Feb 16, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

infomiho Feb 16, 2023 • 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

infomiho Feb 16, 2023 • edited Loading

Choose a reason for hiding this comment

sodic Feb 17, 2023 • 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

infomiho Feb 16, 2023 •

edited

Loading

infomiho Feb 16, 2023 •

edited

Loading

infomiho Feb 16, 2023 •

edited

Loading

sodic Feb 17, 2023 •

edited

Loading