Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Commit

Permalink
Stop beach balling with this one simple trick
Browse files Browse the repository at this point in the history
Fix #10094

The problem is with toJS being called to convert the immutable app state

Instead we just keep everything in Immutable
  • Loading branch information
bbondy committed Aug 5, 2017
1 parent ed41bc8 commit fc814d8
Show file tree
Hide file tree
Showing 7 changed files with 308 additions and 276 deletions.
9 changes: 9 additions & 0 deletions app/common/state/immutableUtil.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,15 @@ const api = {

makeImmutable: (obj) => {
return api.isImmutable(obj) ? obj : Immutable.fromJS(obj)
},

deleteImmutablePaths: (obj, paths) => {
return paths.reduce((result, path) => {
if (path.constructor === Array) {
return obj.deleteIn(path)
}
return obj.delete(path)
}, obj)
}
}

Expand Down
29 changes: 14 additions & 15 deletions app/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,19 +94,19 @@ const defaultProtocols = ['http', 'https']
let loadAppStatePromise = SessionStore.loadAppState()

// Some settings must be set right away on startup, those settings should be handled here.
loadAppStatePromise.then((initialState) => {
loadAppStatePromise.then((initialImmutableState) => {
telemetry.setCheckpointAndReport('state-loaded')
const {HARDWARE_ACCELERATION_ENABLED, SMOOTH_SCROLL_ENABLED, SEND_CRASH_REPORTS} = require('../js/constants/settings')
if (initialState.settings[HARDWARE_ACCELERATION_ENABLED] === false) {
if (initialImmutableState.getIn('settings', HARDWARE_ACCELERATION_ENABLED) === false) {
app.disableHardwareAcceleration()
}
if (initialState.settings[SEND_CRASH_REPORTS] !== false) {
if (initialImmutableState.getIn(['settings', SEND_CRASH_REPORTS]) !== false) {
console.log('Crash reporting enabled')
CrashHerald.init()
} else {
console.log('Crash reporting disabled')
}
if (initialState.settings[SMOOTH_SCROLL_ENABLED] === false) {
if (initialImmutableState.getIn(['settings', SMOOTH_SCROLL_ENABLED]) === false) {
app.commandLine.appendSwitch('disable-smooth-scrolling')
}
})
Expand Down Expand Up @@ -158,18 +158,17 @@ app.on('ready', () => {
appActions.networkDisconnected()
})

loadAppStatePromise.then((initialState) => {
loadAppStatePromise.then((initialImmutableState) => {
// Do this after loading the state
// For tests we always want to load default app state
const loadedPerWindowState = initialState.perWindowState
delete initialState.perWindowState
const loadedPerWindowImmutableState = initialImmutableState.get('perWindowState')
initialImmutableState = initialImmutableState.delete('perWindowState')
// Restore map order after load
let state = Immutable.fromJS(initialState)
appActions.setState(state)
setImmediate(() => perWindowStateLoaded(loadedPerWindowState))
appActions.setState(initialImmutableState)
setImmediate(() => perWindowStateLoaded(loadedPerWindowImmutableState))
})

const perWindowStateLoaded = (loadedPerWindowState) => {
const perWindowStateLoaded = (loadedPerWindowImmutableState) => {
// TODO(bridiver) - this shold be refactored into reducers
// DO NOT ADD ANYHING TO THIS LIST
// See tabsReducer.js for app state init example
Expand All @@ -184,12 +183,12 @@ app.on('ready', () => {
AdInsertion.init()
PDFJS.init()

if (!loadedPerWindowState || loadedPerWindowState.length === 0) {
if (!loadedPerWindowImmutableState || loadedPerWindowImmutableState.size === 0) {
if (!CmdLine.newWindowURL()) {
appActions.newWindow()
}
} else {
loadedPerWindowState.forEach((wndState) => {
loadedPerWindowImmutableState.forEach((wndState) => {
appActions.newWindow(undefined, undefined, wndState)
})
}
Expand Down Expand Up @@ -304,12 +303,12 @@ app.on('ready', () => {
// DO NOT TO THIS LIST - see above

// We need the initial state to read the UPDATE_TO_PREVIEW_RELEASES preference
loadAppStatePromise.then((initialState) => {
loadAppStatePromise.then((initialImmutableState) => {
updater.init(
process.platform,
process.arch,
process.env.BRAVE_UPDATE_VERSION || app.getVersion(),
initialState.settings[settings.UPDATE_TO_PREVIEW_RELEASES]
initialImmutableState.getIn('settings', settings.UPDATE_TO_PREVIEW_RELEASES)
)

// This is fired by a menu entry
Expand Down
Loading

0 comments on commit fc814d8

Please sign in to comment.