jcoreio / crater

Meteor/Webpack/React SSR app skeleton that runs your app code outside of Meteor Isobuild
ISC License
82 stars 10 forks source link

"Meteor code must always run within a Fiber" Production Server SSR only. #162

Open unski11ed opened 7 years ago

unski11ed commented 7 years ago

Hi

I uploaded my app written on top of "crater" to my remote machine for the first time. Everything worked fine in local dev and local production, but of course not on the server. After navigating to the page I see a white screen with "Meteor code must always run within a Fiber. Try wrapping callbacks that you pass to non-Meteor libraries with Meteor.bindEnvironment."

I found out that it only happens when SSR is enabled which is kind of a problem for me, as I really need it.

The call stack points to a thunk where a Meteor method is beeing called:

export function changeLocale(langCode: ?string): Action {
    return (dispatch: Function, currentState: TLocaleState) => {
        const targetLangCode = langCode || cookies.get('langCode') || 'en_GB';  
        const cachedLocale = _.find(currentState.translations, { langCode: targetLangCode });

        if(cachedLocale) {
            dispatch(setCurrentLocale(cachedLocale));
        } else {
            // Notify loading has started
            dispatch(setLoadState(ActionState.Loading));

            Meteor.call('getLocale', targetLangCode, function(error: TMeteorError, phrases: any) {
                const localeDef = _.find(availableLocales, { langCode: targetLangCode });
                const targetLocale = {
                    ...localeDef,
                    phrases
                }

                // Add loaded locale to cache for next changes
                dispatch(addLocaleToCache(targetLocale));
                // Set it as current
                dispatch(setCurrentLocale(targetLocale));
                // Finish Loading
                dispatch(setLoadState(ActionState.Idle));
            });  
        }
    };
}

I don't really get this whole Fiber thing, but i guess the problem has something to do with fiber + dispatching actions in Redux. Can you somehow point me in the right direction in this case? Should I use some specific dispatches for this?

jedwards1211 commented 7 years ago

You know, I just realized that wrapping an async function like createSSR in Meteor.bindEnvironment isn't going to work, because even though the start of its code will get called within a fiber, code after the first await probably isn't because it's running in a callback to the await promise.

This is why meteor-promise was created, it runs its callbacks within Fibers, so I'll have to figure out how to use it in place of the Promise polyfill from babel-runtime. Since the call to renderApp comes after an await statement, this is why you're seeing the error.

node-fibers adds limited concurrency to the V8 Javascript engine. I think as async/await becomes easier to use, we'll see fewer people giving into temptation to use something like fibers instead of staying within the JS single-threaded asynchronous paradigm.

It's also bizarre to me that Meteor chose to use it, because their goal in general was to enable isomorphic code, but when you pick a library that only runs on the server and reaches its tendrils into everything, it kind of gets in the way of isomorphism...

unski11ed commented 7 years ago

Man it's my first time with Meteor, and there are so many bizzare things everywhere. I don't even know now if it saved me any time on development :P

You sound quite certain with your thoughts. Will you be implementing it in the near future?

In the mean time i'll try to hack something out of it or evantually I'll disable SSR for now (which will probably be the case).

jedwards1211 commented 7 years ago

Heh, I hope you didn't get the impression that Crater is the only way to use Meteor! I tried my best to warn about unexpected difficulties like this in the README.

Yeah, this evening I'll figure out how to use meteor-promise. Its tests demonstrate usage: https://github.com/meteor/promise/blob/1e52f297b02ea83e7fb48ba4c2b17d3b4503c001/test/tests.js#L2-L5

unski11ed commented 7 years ago

Yeah yeah, I know, I've seen the notice. Nevertheless the stack looked too good not to try it out :D When you get to know the quirks of meteor, crater works really well! Next time maybe I would just use TypeScript instead of flow, but that's all I'd change for now.

Anyway, great Thanks for the response, and good luck with the update :)

jedwards1211 commented 7 years ago

@unski11ed hmmm...in my workspace at least it looks like Meteor is already monkeypatching Promise to run callbacks in Fibers, so I'm not sure if the cause of your issue is what I thought...

Can you paste the stack trace?

jedwards1211 commented 7 years ago

Heh, upon further investigation (I've just had this error come up in my own project, happy happy joy joy), I realized something worse; I'm stupidly using node_modules/fibers, which is a separate instance than build/meteor/bundle/programs/server/node_modules/fibers. I think that ends up creating a separate instance of Fiber than Meteor uses, and when I run code via the separate instance, its current property is set but the current property on the Fiber that Meteor uses isn't, which causes Meteor to throw the error.

It may be the case that if I remove fibers from build/meteor/bundle/programs/server/package.json after calling meteor build, then Meteor will load fibers from the root node_modules, but I'd rather play it safe by avoiding import 'fibers' anywhere in Crater code and only using Meteor.bindEnvironment, which calls the wrapped function in a Fiber (Meteor's instance of it) if it isn't running in one already. Hopefully, that will fix your issue. I'll need you to check after I commit the fix.

unski11ed commented 7 years ago

Oh, I had a similar problem with 'simpl-schema' package - it was confilicting between the client and meteor, can't remember correctly.

Ok waiting for the patch, I tried some other stuff: cleaned the code a bit, fibered this and that, but nothing worked so far.

jedwards1211 commented 7 years ago

I found yet another potential issue: I'm using babel-runtime and its Promise polyfill from root node_modules, whereas Meteor is using its own dupe of babel-runtime and only monkeypatching the Promise in that. The solution seems to be to make Meteor's Promise global on both client and server, but I'm trying to investigate the cleanest way to avoid dupe packages between Meteor and the root project...

jedwards1211 commented 7 years ago

Okay, can you try the module-resolver branch? It looks in the Meteor bundle's node_modules before looking in the root node_modules so that there are as few duplicate packages as possible.

jedwards1211 commented 7 years ago

@unski11ed I merged that branch now, let me know if master solves your issue.

jedwards1211 commented 7 years ago

@unski11ed any updates on this?

unski11ed commented 6 years ago

@jedwards1211 Sorry for the delay, I left SSR for the last step in my project, which quite frankly is now ;)

I remade this seed project so it matches my requirements and updated some packages along the way - webpack 3, meteor 1.6.1 (last one using babel 6). After some struggling with forcing fiber to compile properly it started running.

Later I started working on the SSR part - I couldn't really just git pull your changes, so I traversed through your commits and made it working - your previous notes in this topic were really helpful. For someone following this path - make sure your code uses the same fiber instance as meteor, like mentioned earlier ;)

Now the server rendering part works ok - I can see that the HTML contains everything what I need BUT i have an error on the client: Cannot find module "../../../../build/meteor/bundle/programs/server/npm/node_modules/babel-runtime/regenerator.

The package is present in this directory, but babel somehow can't import it I guess. Can you give me some hints how can I mitigate this problem and/or point where to look for the solution?