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

Resolve one tick flash problem with async components. #72

Closed
stereobooster opened this issue Dec 27, 2017 · 7 comments
Closed

Resolve one tick flash problem with async components. #72

stereobooster opened this issue Dec 27, 2017 · 7 comments

Comments

@stereobooster
Copy link
Owner

stereobooster commented Dec 27, 2017

Discussed here errorception/snapshotify#1

@stereobooster
Copy link
Owner Author

Possible solution. It is a bit clumsy, but this is just a start.

Assume we collected all async modules upfront and we have is an array.

window.allRequiredModules = ["./a.js", "./b.js"]

Now we can do something like this

const allRequiredModules = window.allRequiredModules;
// Allow this to be garbage-collected
delete window.allRequiredModules;

Promise.all(allRequiredModules.map(x => import(x)))
  .then(modules => {
    hydrete(<App modules={modules} />, root);
  });

And in async component, you do something like this

import loadable from 'loadable-components'

export const Home = loadable(() => import('./a.js'), {
  LoadingComponent: (props) => props.modules['./a.js'] || (<div>Loading...</div>),
})

cc @rakeshpai

@rakeshpai
Copy link

That's very interesting, especially the bit about setting the LoadingComponent to be equal to the cached module when possible. It's certainly worth a shot. I have a feeling that the allRequiredModules.map(x => import(x)) might give some trouble, especially after compilation, but will need to check to be sure.

@rakeshpai
Copy link

Also, I'm trying to understand what's being discussed here facebook/react#1739, but it seems relevant to the issue we're facing.

@stereobooster
Copy link
Owner Author

stereobooster commented Dec 28, 2017

I was wondering why react-loadable needs modules key.

Loadable({
  loader: () => import('./Bar'),
  modules: ['./Bar'],
  webpack: () => [require.resolveWeak('./Bar')],
});

I suppose because of this issue. If yes, then this problem is solved in react-loadable for SSR scenario

UPD: checked source code - it seems those keys used for other purpose.

stereobooster added a commit to stereobooster/an-almost-static-stack that referenced this issue Dec 28, 2017
@stereobooster
Copy link
Owner Author

stereobooster commented Dec 28, 2017

@rakeshpai I actually can not reproduce this error. Here is the branch https://github.com/stereobooster/an-almost-static-stack/tree/experiment-one-tick-flash

code

const Countries = loadable(
  () => {
    const x = import("./views/Countries");
    console.log(1);
    x.then(y => console.log(y));
    console.log(2);
    return x;
  },
  {
    LoadingComponent: () => {
      console.log("LoadingComponent: ./views/Countries");
      return null;
    }
  }
);

console output

1
2
{default: ƒ, __esModule: true}
componentDidMount loaded

there is no LoadingComponent: ./views/Countries

@rakeshpai
Copy link

You're right, I can't replicate it either. Looks like this issue is essentially solved by loadable-components.

I wonder why I couldn't get loadable-components to work for me. I'll need to dig into it to figure out what I've been doing wrong.

I guess this issue can be closed here.

PS: I'll be AFK for the next few days. Will be back on the 2nd. Sorry for the silence for the next few days.

@stereobooster
Copy link
Owner Author

@rakeshpai no worries. Have a nice holidays (if that is why you'll be offline)

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

No branches or pull requests

2 participants