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.

Sign up for GitHub

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

fix(compass-shell): Don't lose the isOperationInProgress state when the user switches tabs, bump mongosh to 2.3.6 COMPASS-8576 COMPASS-8689 #6538

Merged
merged 12 commits into from
Dec 17, 2024
Merged

fix(compass-shell): Don't lose the isOperationInProgress state when the user switches tabs, bump mongosh to 2.3.6 COMPASS-8576 COMPASS-8689 #6538

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
e498226
Don't lose the isOperationInProgress state when the user switches tabs
lerouxb Nov 28, 2024
7e41b55
initial rather
lerouxb Dec 4, 2024
c812b2a
turn shell into a function component
lerouxb Dec 4, 2024
c1ed55c
initialText
lerouxb Dec 11, 2024
fec2872
remove the single connection tab component
lerouxb Dec 12, 2024
aec4757
fix initial in progress
lerouxb Dec 12, 2024
15b54e9
fix initial evaluate
lerouxb Dec 12, 2024
e89ba06
Merge branch 'main' into shell-tab-switch
lerouxb Dec 16, 2024
088fa7e
bump mongosh to 2.3.6
lerouxb Dec 16, 2024
a58abac
don't unhoist
lerouxb Dec 16, 2024
065b2ae
depcheck
lerouxb Dec 16, 2024
b144acd
clarify
lerouxb Dec 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 49 additions & 27 deletions packages/compass-shell/src/components/compass-shell/tab-compass-shell.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,25 +58,17 @@ type CompassShellProps = {
onHistoryChange: (history: string[]) => void;
initialEvaluate?: string | string[];
initialInput?: string;
isOperationInProgress: boolean;
onOperationStarted: () => void;
onOperationEnd: () => void;
};

function useInitialEval(initialEvaluate?: string | string[]) {
const [initialEvalApplied, setInitialEvalApplied] = useTabState(
'initialEvalApplied',
false
);
useEffect(() => {
setInitialEvalApplied(true);
}, [setInitialEvalApplied]);
return initialEvalApplied ? undefined : initialEvaluate;
}

const Shell = React.forwardRef<ShellType, ShellProps>(function Shell(
{ initialEvaluate: _initialEvaluate, ...props },
{ ...props },
ref
) {
const shellRef = useRef<ShellType | null>(null);
const initialEvaluate = useInitialEval(_initialEvaluate);

const mergeRef = useCallback(
(shell: ShellType | null) => {
shellRef.current = shell;
Expand All @@ -93,26 +85,55 @@ const Shell = React.forwardRef<ShellType, ShellProps>(function Shell(
shellRef.current?.focusEditor();
});
}, []);

return <_Shell ref={mergeRef} {...props}></_Shell>;
});

function useInitialEval(initialEvaluate?: string | string[]) {
const [initialEvalApplied, setInitialEvalApplied] = useTabState(
'initialEvalApplied',
false
);
useEffect(() => {
setInitialEvalApplied(true);
}, [setInitialEvalApplied]);
return initialEvalApplied ? undefined : initialEvaluate;
}

const normalizeInitialEvaluate = (initialEvaluate: string | string[]) => {
return (
<_Shell
ref={mergeRef}
initialEvaluate={initialEvaluate}
{...props}
></_Shell>
Array.isArray(initialEvaluate) ? initialEvaluate : [initialEvaluate]
).filter((line) => {
// Filter out empty lines if passed by accident
return !!line;
});
};

const isInitialEvaluateEmpty = (
initialEvaluate?: string | string[] | undefined
) => {
return (
!initialEvaluate || normalizeInitialEvaluate(initialEvaluate).length === 0
);
});
};

const CompassShell: React.FC<CompassShellProps> = ({
runtime,
initialHistory,
onHistoryChange,
initialEvaluate,
initialEvaluate: _initialEvaluate,
initialInput,
}) => {
const initialEvaluate = useInitialEval(_initialEvaluate);
lerouxb marked this conversation as resolved.
Show resolved Hide resolved

const [isOperationInProgress, setIsOperationInProgress] = useTabState(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would probably be nice to have this boolean even higher up so we can change the icon in the tab itself. That way you can see if the operation finishes while you're on a different tab, but that's probably a job for a different ticket.

'isOperationInProgress',
!isInitialEvaluateEmpty(initialEvaluate)
);

const enableShell = usePreference('enableShell');
const shellRef: ShellRef = useRef(null);
const [infoModalVisible, setInfoModalVisible] = useState(false);
const [isOperationInProgress, setIsOperationInProgress] = useState(false);
const [shellOutput, setShellOutput] = useTabState<
readonly ShellOutputEntry[]
>('shellOutput', []);
Expand Down Expand Up @@ -148,13 +169,13 @@ const CompassShell: React.FC<CompassShellProps> = ({
[setShellOutput]
);

const notifyOperationStarted = useCallback(() => {
const onOperationStarted = useCallback(() => {
setIsOperationInProgress(true);
}, []);
}, [setIsOperationInProgress]);

const notifyOperationEnd = useCallback(() => {
const onOperationEnd = useCallback(() => {
setIsOperationInProgress(false);
}, []);
}, [setIsOperationInProgress]);

const canRenderShell = enableShell && initialHistory && runtime;

Expand Down Expand Up @@ -212,8 +233,9 @@ const CompassShell: React.FC<CompassShellProps> = ({
onHistoryChanged={(history) => {
onHistoryChange([...history]);
}}
onOperationStarted={notifyOperationStarted}
onOperationEnd={notifyOperationEnd}
onOperationStarted={onOperationStarted}
onOperationEnd={onOperationEnd}
isOperationInProgress={isOperationInProgress}
maxOutputLength={1000}
maxHistoryLength={1000}
/>
Expand Down
8 changes: 8 additions & 0 deletions packages/compass/webpack.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,12 @@ module.exports = (_env, args) => {
},
};

const snapshot = {
unmanagedPaths: [
path.resolve('..', '..', 'node_modules', '@mongosh', 'browser-repl'),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is useful in combination with packages/browser-repl/scripts/sync-to-compass.js added to mongosh here.

],
};

// Having runtime outside of entries means less rebuilding when dependencies
// change (default is runtime is part of the entry and the whole entry needs
// a rebuild when dependency tree changes)
Expand Down Expand Up @@ -121,6 +127,7 @@ module.exports = (_env, args) => {
return [
merge(mainConfig, {
cache,
snapshot,
externals,
plugins: [
new webpack.EnvironmentPlugin(hadronEnvConfig),
Expand All @@ -137,6 +144,7 @@ module.exports = (_env, args) => {
}),
merge(rendererConfig, {
cache,
snapshot,
// Chunk splitting makes sense only for renderer processes where the
// amount of dependencies is massive and can benefit from them more
optimization,
Expand Down
Loading