Skip to content
This repository was archived by the owner on Jun 28, 2021. It is now read-only.

Conversation

@mmahalwy
Copy link
Contributor

@mmahalwy mmahalwy commented Nov 16, 2017

Title of change

Upgrading to react-router 4

Checklist

  • Unit tests written
  • Manually tested
  • New dependencies were added with yarn
  • Prettier & ESLint were run

Screenshot

insert screenshot if needed

@mmahalwy
Copy link
Contributor Author

mmahalwy commented Nov 16, 2017

@sabeurthabti wanted to get your thoughts on how to handle the server-side stuff and code splitting. So far, this is a pain. I previously used loadable-components here: https://github.com/quran/quran.com-frontend/pull/867/files#diff-cf27c1d543e886c89cd9ac8b8aeaf05b

@ahmedre
Copy link
Contributor

ahmedre commented Nov 16, 2017

Deployed to: http://staging.quran.com:32874

@thabti
Copy link
Contributor

thabti commented Nov 16, 2017

@mmahalwy I will be looking into this more tomorrow iA

@mmahalwy
Copy link
Contributor Author

@sabeurthabti okay, this is working. I don't want to make this PR any larger and feel like a follow up PR can follow to polish. I think this is a good place to stop

@mmahalwy mmahalwy changed the title WIP: Upgrading to RR4 Upgrading to react-router 4 Nov 17, 2017
@thabti
Copy link
Contributor

thabti commented Nov 17, 2017

Tbh I have not done much server-rendering recently, so I wouldn't have had much input.
Glad it is working.

LGTMI!

@mmahalwy
Copy link
Contributor Author

rebuild

@mmahalwy
Copy link
Contributor Author

@ahmedre I think jenkins died again :I

@ahmedre
Copy link
Contributor

ahmedre commented Nov 22, 2017

rebuild

@ahmedre
Copy link
Contributor

ahmedre commented Nov 22, 2017

thanks for the heads up - should be good now

@ahmedre
Copy link
Contributor

ahmedre commented Nov 22, 2017

Deployed to: http://staging.quran.com:32875

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants