Skip to content

Commit

Permalink
Fix/remounting (#577)
Browse files Browse the repository at this point in the history
* Enable our old emmet mechanism as well

* Find culprit of full remount on work

* Fix remounting routes

* Clean up diff

* Fix route changes

* Fix loading other sandboxes

* Move load statement
  • Loading branch information
CompuIves authored Feb 21, 2018
1 parent 7650b58 commit c57271d
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 7 deletions.
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@
"build": "yarn build:dependents && yarn build:prod",
"build:prod": "lerna run build --scope app --scope homepage --parallel && gulp",
"build:dependents": "lerna run build --ignore app --ignore homepage --parallel",
"start": "lerna run start --parallel",
"start": "lerna run build --scope codesandbox-browserfs --stream && lerna run start --parallel",
"start:fast": "lerna run start --scope app --stream",
"start:test": "lerna run start:test --scope app --stream",
"start:dev_api": "lerna run start --parallel --ignore app & lerna run start:dev_api --scope app --stream",
"test": "lerna run test --ignore codesandbox-browserfs",
Expand Down
13 changes: 10 additions & 3 deletions packages/app/config/webpack.common.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,7 @@ const ESLINT_PLUGIN_VUE_INDEX = `module.exports = {
.filter(filename => path.extname(filename) === '.js')
.map(filename => {
const ruleId = path.basename(filename, '.js');
return ` "${ruleId}": require("eslint-plugin-vue/lib/rules/${
filename
}"),`;
return ` "${ruleId}": require("eslint-plugin-vue/lib/rules/${filename}"),`;
})
.join('\n')}
},
Expand Down Expand Up @@ -147,6 +145,15 @@ module.exports = {
flags: 'g',
},
},
// Remove dynamic require in jest circus
{
test: /format_node_assert_errors\.js/,
loader: 'string-replace-loader',
options: {
search: `assert = require.call(null, 'assert');`,
replace: `throw new Error('module assert not found')`,
},
},
// JSON is not enabled by default in Webpack but both Node and Browserify
// allow it implicitly so we also enable it.
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ class Project extends React.Component {
<PropertyName>Forked From</PropertyName>
<PropertyValue>
<ConfirmLink
enabled={store.editor.isAllModulesSynced}
enabled={!store.editor.isAllModulesSynced}
message="You have unsaved changes. Are you sure you want to navigate away?"
to={sandboxUrl(sandbox.forkedFromSandbox)}
>
Expand Down
10 changes: 8 additions & 2 deletions packages/app/src/app/pages/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import * as React from 'react';
import { inject, observer } from 'mobx-react';
import Loadable from 'react-loadable';
import { Route, Switch, Redirect, withRouter } from 'react-router-dom';
import { Route, Switch, Redirect } from 'react-router-dom';

import _debug from 'app/utils/debug';
import Notifications from 'app/pages/common/Notifications';
Expand Down Expand Up @@ -68,6 +68,12 @@ class Routes extends React.Component<Props> {
this.props.signals.appUnmounted();
}

shouldComponentUpdate() {
// Without this the app won't update on route changes, we've tried using
// `withRouter`, but it caused the app to remount on every route change.
return true;
}

render() {
return (
<Container>
Expand Down Expand Up @@ -110,4 +116,4 @@ class Routes extends React.Component<Props> {
}
}

export default inject('signals', 'store')(withRouter(observer(Routes)));
export default inject('signals', 'store')(observer(Routes));

0 comments on commit c57271d

Please sign in to comment.