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.

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

Thoughts #19

Open
craigspaeth opened this issue Jan 5, 2017 · 4 comments
Open

Thoughts #19

craigspaeth opened this issue Jan 5, 2017 · 4 comments

Comments

@craigspaeth
Copy link

craigspaeth commented Jan 5, 2017

Not sure an issue is the right format to leave my thoughts on this, but I'll quickly jot them down here:

  • Firstly, this is awesome—I very much support the direction this is going
  • Not something for now, but opportunity for later—we could probably wrap up a fair bit of boilerplate in modules/framework-ey stuff. For instance, there's a lot of noise here to basically say "render a component on the #root element".
  • These HTML strings are kinda ugly, could we use stateless react components instead?
  • For bigger projects I'd encourage a modular architecture. E.g. what we do in Force/MG where you have sub-apps and a components directories that make it easy to say "fork" the artist page by copying the sub-app and splitting between the two at a routing level. The co-location of files also makes it easy to experiment and drop old code. I also think this provides obvious "escape hatches" for when one finds themselves fighting the React/Relay/etc. stack, e.g. they could implement a sub-app that does more vanilla Express stuff or introduces a new approach to UI suited to that edge case need.
  • Might go without saying, but config like this should be moved to environment variables and a .env file
  • Be careful with doing universal stuff with Webpack, because some of the Webpack loader features can make server-side rendering a pain
  • It might be good to be able to toggle off hot module loading (e.g. with an env var), b/c it can get kinda magical and full of edge cases (ideally we'd avoid having if (module.hot) clutter the code too)
  • It'll be interesting to see how the testing story shapes up after some of the architecture settles
  • Eventually would be good to include docs on the whole VSCode/tooling setup
  • I need to spend more time with Relay, but I wonder what a more client-side complex app will look like and how one would manage "application state" over the data fetching story. I'm also curious how much value we're getting out of Relay when we expect to mainly do server-side rendering—which I expect would typically involve pretty simple fetch().then(() => res.send(ReactComponent)) code. I guess the component portability/static analysis?
  • Have we considered any universal routing solutions like React Router (not suggesting it, just curious)?

👍 👍 👍 exciting stuff!!

@acjay
Copy link

acjay commented Jan 9, 2017

I was going to chime in about React Router. To be fair, it gets mixed reviews from the API churn it has had over the past year or so. But I do think that the impact of those changes is quite overblown, and I also think it's a pretty good choice for being able to share routes between client and server, and it's pretty simple to use. I think of it as a declarative way to select the layout of the page from the route, rather than the more imperative Express flow, which also doesn't map as well to the client, AFAIK. There are probably other good options as well, but I'm not highly familiar.

@alloy
Copy link
Owner

alloy commented Jan 13, 2017

  • Not something for now, but opportunity for later—we could probably wrap up a fair bit of boilerplate in modules/framework-ey stuff. For instance, there's a lot of noise here to basically say "render a component on the #root element".
  • These HTML strings are kinda ugly, could we use stateless react components instead?
  • Might go without saying, but config like this should be moved to environment variables and a .env file
  • It might be good to be able to toggle off hot module loading (e.g. with an env var), b/c it can get kinda magical and full of edge cases (ideally we'd avoid having if (module.hot) clutter the code too)

Totally 👍 These are currently all only consequences of getting the tech we want to experiment with up and running as fast as possible.

  • For bigger projects I'd encourage a modular architecture. E.g. what we do in Force/MG where you have sub-apps and a components directories that make it easy to say "fork" the artist page by copying the sub-app and splitting between the two at a routing level. The co-location of files also makes it easy to experiment and drop old code. I also think this provides obvious "escape hatches" for when one finds themselves fighting the React/Relay/etc. stack, e.g. they could implement a sub-app that does more vanilla Express stuff or introduces a new approach to UI suited to that edge case need.

Makes a lot of sense 👍 #20

  • Be careful with doing universal stuff with Webpack, because some of the Webpack loader features can make server-side rendering a pain

Totally, I’m not adding any Webpack stuff that I don’t fully understand or won’t work with our setup.

  • Eventually would be good to include docs on the whole VSCode/tooling setup

Agreed. @orta wanna pick that up?

  • Have we considered any universal routing solutions like React Router (not suggesting it, just curious)?

Thus far my only consideration has been that I don’t want to introduce any of that until there’s a good reason to do so. I would prefer all routing to happen server-side. In my opinion it makes it easier to decide what type of routing to do when, where routing is performed, and just simplifies reasoning about the app in general.

@alloy
Copy link
Owner

alloy commented Jan 13, 2017

I need to spend more time with Relay, but I wonder what a more client-side complex app will look like and how one would manage "application state" over the data fetching story.

What “state” are you thinking of?

From what I’ve seen, Force has very little client-side state. The bits it has can probably all live in a component’s state dictionary.

I'm also curious how much value we're getting out of Relay when we expect to mainly do server-side rendering—which I expect would typically involve pretty simple fetch().then(() => res.send(ReactComponent)) code. I guess the component portability/static analysis?

Code locality, the data requirements are in the same file as the component that needs it. The components don’t have to deal with actually fetching that data and optimising multiple queries into 1.

Whether this happens on the server or the client makes little difference.

(Currently we serialise the Relay request data so that the client doesn’t need to make any unnecessary requests as well, but it’s not entirely clear how/if that would work in Relay 2.)

The static analysis component applies somewhat right now, but it’s going to play a much bigger role in Relay 2.

@craigspaeth
Copy link
Author

craigspaeth commented Jan 13, 2017

Yeah generally Force doesn't have a lot of client-side state, but occasionally it will have quite a bit of it (e.g. https://www.artsy.net/collect, inquiry modal flows, other onboarding/set-management style interfaces). I think leaving this to component's internal state is a totally good default and I'm happy to have Relay's co-location + query aggregating benefits 👍 Thanks for explaining!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants