Skip to content

Commit

Permalink
WIP prevent stale references to active tab
Browse files Browse the repository at this point in the history 
* activeTab is now activeTabId
* use getActiveTab to get active tab
* getTabComponent refactored for understandability
  • Loading branch information
@philippfromme
philippfromme committed Feb 7, 2025
1 parent a747e27 commit b16703d
Showing 1 changed file with 112 additions and 72 deletions.
184 changes: 112 additions & 72 deletions client/src/app/App.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ const FILTER_ALL_EXTENSIONS = {
};

const INITIAL_STATE = {
activeTab: EMPTY_TAB,
activeTabId: EMPTY_TAB.id,
dirtyTabs: {},
unsavedTabs: {},
recentTabs: [],
Expand Down Expand Up @@ -156,6 +156,34 @@ export class App extends PureComponent {
this.currentNotificationId = 0;
}

/**
* Get tab from state.
*
* @param {string} tabId
*
* @returns {Tab}
*/
getTab = (tabId) => {
const { tabs } = this.state;

return tabs.find(tab => tab.id === tabId);
};

/**
* Get active tab from state.
*
* @returns {Tab}
*/
getActiveTab = () => {
const { activeTabId } = this.state;

if (activeTabId === EMPTY_TAB.id) {
return EMPTY_TAB;
}

return this.getTab(activeTabId);
};

createDiagram = async (type = 'bpmn') => {

const {
Expand All @@ -177,10 +205,9 @@ export class App extends PureComponent {
addTab(tab, properties = {}) {

this.setState((state) => {
const {
tabs,
activeTab
} = state;
const { tabs } = state;

const activeTab = this.getActiveTab();

if (tabs.indexOf(tab) !== -1) {
throw new Error('tab exists');
Expand Down Expand Up @@ -213,11 +240,9 @@ export class App extends PureComponent {
* Navigate shown tabs in given direction.
*/
navigate(direction) {
const { tabs } = this.state;

const {
activeTab,
tabs
} = this.state;
const activeTab = this.getActiveTab();

// next tab in line as a fallback to history
// navigation
Expand Down Expand Up @@ -300,13 +325,8 @@ export class App extends PureComponent {
assign(updatedTab, newAttrs);

this.setState((state) => {
const { tabs } = state;

const {
activeTab,
tabs
} = state;

// replace in tabs
const updatedTabs = tabs.map(t => {
if (t === tab) {
return updatedTab;
Expand All @@ -315,16 +335,8 @@ export class App extends PureComponent {
return t;
});


// replace activeTab
let updatedActiveTab = activeTab;
if (activeTab.id === updatedTab.id) {
updatedActiveTab = updatedTab;
}

return {
...newState,
activeTab: updatedActiveTab,
tabs: updatedTabs
};
});
Expand All @@ -344,11 +356,13 @@ export class App extends PureComponent {
* @return {Promise<Void>} tab shown promise
*/
showTab(tab) {
if (!this.getTab(tab.id) === tab) {
throw new Error('tab does not exist');
}

const {
activeTab,
tabShown
} = this.state;
const { tabShown } = this.state;

const activeTab = this.getActiveTab();

if (activeTab === tab) {
return tabShown.promise;
Expand All @@ -366,9 +380,8 @@ export class App extends PureComponent {

const deferred = pDefer();


this.setState({
activeTab: tab,
activeTabId: tab.id,
Tab: this.getTabComponent(tab),
tabShown: deferred,
tabState: {},
Expand Down Expand Up @@ -494,10 +507,11 @@ export class App extends PureComponent {

const {
tabs,
activeTab,
openedTabs
} = this.state;

const activeTab = this.getActiveTab();

const {
navigationHistory,
closedTabs,
Expand Down Expand Up @@ -574,9 +588,7 @@ export class App extends PureComponent {
tabsProvider
} = this.props;

const {
activeTab
} = this.state;
const activeTab = this.getActiveTab();

const providers = tabsProvider.getProviders();

Expand Down Expand Up @@ -699,10 +711,11 @@ export class App extends PureComponent {
// open the tab for the desired file or, if not found,
// the last opened tab
if (activateFile !== false) {
const activeTab = activateFile && this.findOpenTab(activateFile) || openedTabs[openedTabs.length - 1];
const tabToSelect = activateFile && this.findOpenTab(activateFile) || openedTabs[openedTabs.length - 1];


if (activeTab) {
await this.selectTab(activeTab);
if (tabToSelect) {
await this.selectTab(tabToSelect);
}
}

Expand Down Expand Up @@ -861,10 +874,11 @@ export class App extends PureComponent {

const {
openedTabs,
activeTab,
tabShown
} = this.state;

const activeTab = this.getActiveTab();

if (tab === activeTab) {
tabShown.resolve();
} else {
Expand Down Expand Up @@ -1052,34 +1066,60 @@ export class App extends PureComponent {
return tab;
}

/**
* Get tab component for given tab. Load tab component if not cached. Return
* loading tab component if tab component is a promise.
*
* @param {Tab} tab
*
* @returns {Promise<React.Component>|React.Component} tab component
*/
getTabComponent(tab) {
const { type } = tab;

const type = tab.type;

if (this.tabComponentCache[type]) {
return this.tabComponentCache[type];
if (this.tabComponentCache[ type ]) {
return this.tabComponentCache[ type ];
}

const {
tabsProvider
} = this.props;
const { tabsProvider } = this.props;

var tabComponent = tabsProvider.getTabComponent(type) || missingProvider(type);
/** @type {Promise<React.Component>|React.Component} */
const TabComponent = tabsProvider.getTabComponent(type) || getMissingProviderTabComponent(type);

Promise.resolve(tabComponent).then((c) => {
if (TabComponent instanceof Promise) {
TabComponent.then(TabComponent => {
this.setTabComponent(tab, TabComponent);
});

var Tab = c.default || c;
return LoadingTab;
} else {
this.setTabComponent(tab, TabComponent);

this.tabComponentCache[type] = Tab;
return TabComponent;
}
}

if (this.state.activeTab === tab) {
this.setState({
Tab
});
}
});
/**
* Set tab component for given tab. Set tab component as active tab component
* if tab is active tab.
*
* @param {Tab} tab
* @param {{ default: React.Component }|React.Component} TabComponent
*/
setTabComponent(tab, TabComponent) {
const { id, type } = tab;

return (this.tabComponentCache[type] = LoadingTab);
// handle both ES modules and CommonJS
TabComponent = TabComponent.default || TabComponent;

this.tabComponentCache[ type ] = TabComponent;

// update tab component if it is currently active
const activeTab = this.getActiveTab();

if (activeTab.id === id) {
this.setState({ Tab: TabComponent });
}
}

componentDidMount() {
Expand All @@ -1103,22 +1143,24 @@ export class App extends PureComponent {
componentDidUpdate(prevProps, prevState) {

const {
activeTab,
activeTabId,
tabs,
recentTabs,
tabLoadingState,
tabState,
layout
} = this.state;

const activeTab = this.getActiveTab();

const {
onTabChanged,
onTabShown
} = this.props;

if (prevState.activeTab !== activeTab) {
if (prevState.activeTabId !== activeTabId) {
if (typeof onTabChanged === 'function') {
onTabChanged(activeTab, prevState.activeTab);
onTabChanged(activeTab, this.getTab(prevState.activeTabId));
}

this.emit('app.activeTabChanged', {
Expand All @@ -1139,7 +1181,7 @@ export class App extends PureComponent {
}

if (
activeTab !== prevState.activeTab ||
activeTabId !== prevState.activeTabId ||
tabs !== prevState.tabs ||
layout !== prevState.layout
) {
Expand Down Expand Up @@ -1194,10 +1236,11 @@ export class App extends PureComponent {

const {
layout,
tabs,
activeTab
tabs
} = this.state;

const activeTab = this.getActiveTab();

return onWorkspaceChanged({
tabs,
activeTab,
Expand Down Expand Up @@ -1626,7 +1669,7 @@ export class App extends PureComponent {

onMenuUpdate({
...options,
activeTab: this.state.activeTab,
activeTab: this.getActiveTab(),
lastTab: !!this.closedTabs.get(),
closedTabs: this.state.recentTabs,
tabs: this.state.tabs
Expand Down Expand Up @@ -1749,11 +1792,7 @@ export class App extends PureComponent {
}

triggerAction = failSafe((action, options = {}) => {

const {
activeTab
} = this.state;

const activeTab = this.getActiveTab();

log('App#triggerAction %s %o', action, options);

Expand Down Expand Up @@ -1857,13 +1896,13 @@ export class App extends PureComponent {
}

if (action === 'close-active-tab') {
let activeId = this.state.activeTab.id;
let activeId = activeTab.id;

return this.closeTabs(t => t.id === activeId);
}

if (action === 'close-other-tabs') {
let activeId = options && options.tabId || this.state.activeTab.id;
let activeId = options && options.tabId || activeTab.id;

return this.closeTabs(t => t.id !== activeId);
}
Expand Down Expand Up @@ -2009,7 +2048,7 @@ export class App extends PureComponent {
getConfig = (key, ...args) => {
const config = this.getGlobal('config');

const { activeTab } = this.state;
const activeTab = this.getActiveTab();

const { file } = activeTab;

Expand Down Expand Up @@ -2100,11 +2139,12 @@ export class App extends PureComponent {

const {
tabs,
activeTab,
layout,
logEntries
} = this.state;

const activeTab = this.getActiveTab();

const isDirty = (tab) => {
return this.isUnsaved(tab) || this.isDirty(tab);
};
Expand Down Expand Up @@ -2271,7 +2311,7 @@ export class App extends PureComponent {
}


function missingProvider(providerType) {
function getMissingProviderTabComponent(providerType) {
class MissingProviderTab extends PureComponent {

componentDidMount() {
Expand Down

0 comments on commit b16703d

Please sign in to comment.