[Branch: feature/found] Routing with Found #182

LorbusChris · 2016-11-19T18:47:09Z

Uses Found for react routing.

Implements Client & Server Side Rendering with Found.

~~For now, only supports Client Server Rendering with Found.~~

I will update this PR as soon as Found's creator @taion adds an SSR API.
We can then determine, whether or not code-split-component's createRenderContext() will work without additional changes to it.

@ctrlplusb

ctrlplusb · 2016-11-28T00:39:50Z

This is great @LorbusChris! Thanks.

I've been following found myself. Had an interesting chat with @taion about it on twitter. Looks like a really awesome and promising project. I am definitely open to this PR. :)

ctrlplusb · 2016-12-05T10:32:53Z

@LorbusChris I see SSR landed. :)

I would hold off doing any coding on this. A big update is in the pipeline which we should probably wait for before doing any more branch work.

taion · 2016-12-05T10:51:16Z

The Found SSR API is pretty trivial anyway (but it handles waiting for data dependencies for you!).

ctrlplusb · 2016-12-05T10:54:43Z

@taion which is pretty darn amazing! :)

ctrlplusb · 2016-12-07T17:01:56Z

@LorbusChris v10.0.0 is almost ready to ship. :)

LorbusChris · 2016-12-08T02:11:42Z

Good news first: This is working in development now. 🎉

Unfortunately in production the client is not being loaded and I'm getting this error in the node console:

(node:23095) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 2): TypeError: Object is not async iterable
(node:23095) DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

If I'm not mistaken this has to do with this issue with babel-runtime: babel/#4783:. Maybe we can use babel-polyfill here for the time being?

@ctrlplusb I'm not sure what your plans are in regards to branches using Found (or any other routing lib for that matter), thereby deviating from the base-stack's RRv4. In the mid-run I'm going to be steering this towards relay (with found-relay) and graphql. First, however, I'll connect this to keystone, which provides a neat admin UI and API for managing mongoose models and connecting to MongoDB or Redis. If there's any interest in a boilerplate blog/gallery implementation, I'll happily share :)

FYI @taion
Thanks to you both for the fundamental work beneath!

Everyone, constructive reviews are highly appreciated!

LorbusChris · 2016-12-08T02:43:19Z

src/client/index.js

@@ -25,9 +30,7 @@ function renderApp(TheApp) {
    render(


There is a problem with this Promise

taion · 2016-12-08T11:26:07Z

Yeah, it is that bug. Interesting. Pulling in the polyfill explicitly would fix things. Let's see.

ctrlplusb · 2016-12-08T11:29:22Z

@LorbusChris great work. In terms of the branch structure I would prefer if you kept the tech stack as purely "found" for now. We could introduce extended versions of the branch that could introduce additional technologies (i.e. "relay"). :)

I think have a base "found" branch will be great, as it opens itself up for solving data fetching needs for many libraries (e.g. redux).

ctrlplusb · 2016-12-08T11:31:25Z

@taion I think the polyfill.io service allows you to specify additional flags for es6 specific features. I believe slate has a similar need. Perhaps the babel-polyfill isn't a strict requirement.

ctrlplusb · 2016-12-08T11:32:29Z

@LorbusChris @taion

Try the following:

<script src="https://cdn.polyfill.io/v2/polyfill.js?features=es6"></script>

taion · 2016-12-08T11:41:35Z

It's not an ES6 feature. It's stage 3.

ctrlplusb · 2016-12-08T11:42:58Z

Ah oki. 😟

What is the feature? Out of interest.

taion · 2016-12-08T13:24:23Z

It's async iterator support – it's how Found manages displaying loading states while fetching data.

ctrlplusb · 2016-12-08T13:27:10Z

Of course yes. Thanks.

LorbusChris · 2016-12-08T15:37:26Z

@taion Sorry I'm still in the unclear about this. Is this actionable and can be solved by using the polyfill right now?
AFAIK we're already adding the polyfill in generateHTML(). Would this need a static html with the script tag in it?

ctrlplusb · 2016-12-08T15:40:13Z

@LorbusChris Thinking about this. You may just need to add the babel async-to-generator plugin to the project.

LorbusChris · 2016-12-08T16:05:25Z

@ctrlplusb Thanks, I'll have a look into that! (probably not before the weekend though)

Edit: Resolved merge conflict.
Note to self: TODO: Comments, BRANCH.md

taion · 2016-12-08T16:16:24Z

I think babel-polyfill is the way to go pending the runtime fix. It's the symbol polyfill that's broken, not the transpilation.

codepunkt · 2016-12-19T15:20:41Z

If this also handles data fetching and integrates with redux - how does it work with apollo/graphql? :)

ctrlplusb · 2016-12-19T17:45:10Z

Amazing @LorbusChris!!

I can't wait to try it out. Great work!

ctrlplusb · 2016-12-19T17:46:21Z

@codepunkt maybe you could use the client-agnostic APIs of Apollo within the getData bindings?

taion · 2016-12-19T18:09:01Z

I think that's how I'd do the Apollo integration 👍

ctrlplusb · 2016-12-20T09:25:55Z

Hey @LorbusChris we can merge this in as soon as we do the next release from next.

ctrlplusb · 2016-12-20T09:27:26Z

src/shared/components/DemoApp/routeConfig.js

+  return <Component {...props} />;
+}
+
+function CodeSplitHome() {


👍

It's nice wrapping it up like this.

ctrlplusb · 2016-12-20T09:28:04Z

tools/webpack/configFactory.js

@@ -125,14 +125,17 @@ export default function webpackConfigFactory(buildOptions: BuildOptions) {
      // This makes importing of the output module as simple as:
      //   import server from './build/server';
      index: removeEmpty([
+        // Required on the server to fix an issue with Found's transpilation
+        ifNode('core-js/library/fn/symbol'),


👍

Well found! Haha, pardon the pun.

yeah... still need to fix this on the babel side, sigh

ctrlplusb · 2016-12-20T09:29:14Z

Really great work here @LorbusChris :)

I'm excited to get this in

LorbusChris · 2016-12-20T10:45:41Z

@ctrlplusb very glad to hear that! :)

As there's already a merge conflict with the yarn.lock file, I propose the following:

When the next release is out, I'll update the FEATURE.md accordingly and rebase the branch one last time so all changes can be cleanly merged in on top of that release.

Now, with you having already reviewed the PR, please do tell if you'd prefer a merger over a rebase. The diff should stay the same, though.

Implements Client & Server Side Rendering with Found

Adds regenerator-runtime on the server Adds core-js symbol polyfill on the server to fix issue with broken async generator function transpilation in babel-runtime

Adds acknowledgements Improves wording

LorbusChris · 2016-12-24T12:24:31Z

I believe this is ready to merge now 🎉

ctrlplusb · 2016-12-24T12:25:09Z

Let's do it! 👍😀

ctrlplusb · 2016-12-24T12:25:55Z

💫💥💖

ctrlplusb · 2016-12-24T12:26:36Z

We just need to update the feature branches doc on master now. Thanks for this @LorbusChris !

taion · 2017-01-25T18:19:38Z

babel/babel#5195 would also fix the babel-runtime issue, BTW.

ctrlplusb · 2017-01-25T21:03:56Z

Awesome news @taion

Thanks for thinking of us! 🙏

taion · 2017-02-14T19:28:02Z

This should work now with the new babel-runtime 6.23.0.

LorbusChris mentioned this pull request Nov 21, 2016

Add server rendering support 4Catalyzer/found#38

Closed

LorbusChris force-pushed the found branch from ed6f392 to 0b658ee Compare December 7, 2016 23:43

LorbusChris changed the base branch from master to next December 7, 2016 23:43

LorbusChris force-pushed the found branch from 0b658ee to 12938ee Compare December 8, 2016 01:15

LorbusChris force-pushed the found branch from 12938ee to 369cdb8 Compare December 8, 2016 02:33

LorbusChris commented Dec 8, 2016

View reviewed changes

src/client/index.js

@@ -25,9 +30,7 @@ function renderApp(TheApp) {

render(

Copy link

Collaborator Author

LorbusChris Dec 8, 2016

Choose a reason for hiding this comment

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

There is a problem with this Promise

LorbusChris changed the base branch from next to master December 8, 2016 15:13

LorbusChris changed the title ~~[WIP][Branch Idea]Use Found as react routing library.~~ [RFC/WIP][Branch Idea]Use Found as react routing library. Dec 8, 2016

LorbusChris force-pushed the found branch from 369cdb8 to 3de0d0a Compare December 8, 2016 16:06

LorbusChris changed the title ~~[RFC/WIP][Branch Idea]Use Found as react routing library.~~ [RFC/WIP][Branch][Alt-Base]Found Routing Dec 8, 2016

LorbusChris force-pushed the found branch from 3de0d0a to 7f9e9cb Compare December 10, 2016 20:27

ctrlplusb changed the base branch from master to incubator/routing/found December 11, 2016 09:12

LorbusChris changed the title ~~[WIP][Branch feature/found] Routing with Found~~ [Branch: feature/found] Routing with Found Dec 19, 2016

ctrlplusb reviewed Dec 20, 2016

View reviewed changes

LorbusChris force-pushed the found branch from 50957ac to 7fdf782 Compare December 23, 2016 00:04

LorbusChris changed the base branch from next to master December 24, 2016 11:58

LorbusChris and others added 5 commits December 24, 2016 13:03

Use Found for React Routing

ab9f87e

Implements Client & Server Side Rendering with Found

Catches exception in async middleware

f8f3606

SSR

13d30c0

Adds regenerator-runtime on the server Adds core-js symbol polyfill on the server to fix issue with broken async generator function transpilation in babel-runtime

Create FEATURE.md

1b70c0e

Update FEATURE.md

b100e2f

Adds acknowledgements Improves wording

LorbusChris force-pushed the found branch from 7fdf782 to b100e2f Compare December 24, 2016 12:15

ctrlplusb changed the base branch from master to feature/found December 24, 2016 12:16

Updates FEATURE.md and yarn.lock

4ebfebf

ctrlplusb merged commit 5af1884 into ctrlplusb:feature/found Dec 24, 2016

taion mentioned this pull request Jan 16, 2017

Add redux examples 4Catalyzer/found#65

Merged

snyk-bot mentioned this pull request Dec 3, 2020

[Snyk] Upgrade cross-env from 5.0.5 to 5.2.1 sgilliam15/react-universally#4

Open

[Branch: feature/found] Routing with Found #182

[Branch: feature/found] Routing with Found #182

Conversation

LorbusChris commented Nov 19, 2016 • edited

ctrlplusb commented Nov 28, 2016

ctrlplusb commented Dec 5, 2016

taion commented Dec 5, 2016

ctrlplusb commented Dec 5, 2016

ctrlplusb commented Dec 7, 2016

LorbusChris commented Dec 8, 2016 • edited

LorbusChris Dec 8, 2016

Choose a reason for hiding this comment

taion commented Dec 8, 2016

ctrlplusb commented Dec 8, 2016

ctrlplusb commented Dec 8, 2016

ctrlplusb commented Dec 8, 2016

taion commented Dec 8, 2016

ctrlplusb commented Dec 8, 2016

taion commented Dec 8, 2016

ctrlplusb commented Dec 8, 2016

LorbusChris commented Dec 8, 2016

ctrlplusb commented Dec 8, 2016

LorbusChris commented Dec 8, 2016 • edited

taion commented Dec 8, 2016

codepunkt commented Dec 19, 2016

ctrlplusb commented Dec 19, 2016

ctrlplusb commented Dec 19, 2016

taion commented Dec 19, 2016

ctrlplusb commented Dec 20, 2016

ctrlplusb Dec 20, 2016

Choose a reason for hiding this comment

ctrlplusb Dec 20, 2016

Choose a reason for hiding this comment

taion Dec 20, 2016

Choose a reason for hiding this comment

ctrlplusb commented Dec 20, 2016

LorbusChris commented Dec 20, 2016

LorbusChris commented Dec 24, 2016

ctrlplusb commented Dec 24, 2016

ctrlplusb commented Dec 24, 2016

ctrlplusb commented Dec 24, 2016

taion commented Jan 25, 2017

ctrlplusb commented Jan 25, 2017

taion commented Feb 14, 2017

LorbusChris commented Nov 19, 2016 •

edited

LorbusChris commented Dec 8, 2016 •

edited

LorbusChris commented Dec 8, 2016 •

edited