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

Add a plugin to embed React components within TT #502

Merged
merged 31 commits into from May 5, 2017

Conversation

mwiencek
Copy link
Member

@mwiencek mwiencek commented Apr 3, 2017

Based on #500 so it includes those changes for now.

Does what the title says, which I'm pretty sure is an idea @chirlu at least has suggested over a year ago.

It also does the following, broadly:

  • Makes the template renderer communicate with the Perl over a UNIX socket, which is vastly more performant than the current HTTP + Redis combo. Kind of important since we're going to be communicating with it possibly many times per page load, rather than just once.
  • Renders the <head> element on every page using React.
  • Fixes any other related things that affected the development of the above two items.

@mwiencek
Copy link
Member Author

I fixed some bugs related to the new UNIX socket communication, and made the external links/favicons in the sidebar also get rendered by React. Not because I want to stuff more things in this PR (I don't), but I wanted to reduce the amount of data needed to be sent over the socket for each request (which includes the Catalyst stash), and doing that removed FAVICON_CLASSES from the stash, which was a large chunk of it.

So we can use it to write a plugin for embedding React components within
TT.
@mwiencek mwiencek force-pushed the react-in-tt branch 2 times, most recently from efcd7aa to 5aae72f Compare April 14, 2017 19:53
Instead of trying to delete all the problematic keys that contain CODE
references or circular data structures.
Some components don't need context.
We don't want the component path to include the query parameters.
It won't send errors if the DSN is falsey, but not running it will cause
exceptions to be thrown later.
This better allows us to provide mostly the same API.
Now that we have a plugin for embedding React components within TT,
reading & writing data to Redis for every call to that plugin has become
prohibitively slow. Use a UNIX socket to communicate with the renderer
instead.

The template-renderer Dockerfile has been removed, and a renderer
service has been added to the existing website container. This is
simpler and probably more correct, since MBS can only communicate with a
renderer on the same host, and it doesn't make sense to communicate with
it as a standalone service, only through the website.
These variables aren't even in the environment anymore, so this code
didn't work. It was likely broken sometime around
49e48ce.
The effect of this function is to allow for easier development, so
JavaScript files/templates can be edited without having to restart the
renderer (otherwise, the modules and their associated definitions would
be cached).

This function was previously called at the start of each request to the
renderer. But given that we can now emebed React components within TT,
it's incredibly slow to call this for every component (which corresponds
to what used to be a "request"). So, we now only call this function at
the beginning of each request cycle.
Needs to be stringified (React can't output objects).
Note: Having Form::Search::Search and Form::User::Login extend
MusicBrainz::Server::Form breaks those forms in silent and strange ways,
so I've moved TO_JSON to a separate role for now.
@mwiencek mwiencek force-pushed the react-in-tt branch 2 times, most recently from 2cf8aac to 0a58a1c Compare April 15, 2017 17:54
@mwiencek
Copy link
Member Author

mwiencek commented May 5, 2017

Ok, I was hoping to merge this sooner, but let's try to release it with the schema change.

@mwiencek mwiencek merged commit a111db2 into metabrainz:master May 5, 2017
@mwiencek mwiencek deleted the react-in-tt branch May 5, 2017 02:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant