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.

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

Conversation

lerouxb
Copy link
Contributor

@lerouxb lerouxb commented Nov 28, 2024

See also mongodb-js/mongosh#2284

To test this, the easiest one is to connect to our analytics-node-test cluster, then in a shell:

use dob
db.permits.countDocuments({ foo: 1 })

Then switch to another tab like Welcome, then switch back to the shell tab.

Note: This also bumps mongosh to 2.3.6.

@@ -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.

initialInput,
}) => {
const initialEvaluate = useInitialEval(_initialEvaluate);

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.

@lerouxb lerouxb changed the title chore(compass-shell): Don't lose the isOperationInProgress state when the user switches tabs COMPASS-8576 fix(compass-shell): Don't lose the isOperationInProgress state when the user switches tabs COMPASS-8576 Nov 28, 2024
@github-actions github-actions bot added the fix label Nov 28, 2024
@lerouxb lerouxb changed the title fix(compass-shell): Don't lose the isOperationInProgress state when the user switches tabs COMPASS-8576 fix(compass-shell): Don't lose the isOperationInProgress state when the user switches tabs, bump mongosh to 2.3.6 COMPASS-8576 COMPASS-8689 Dec 16, 2024
@@ -8560,231 +8560,6 @@
"resolved": "configs/webpack-config-compass",
"link": true
},
"node_modules/@mongosh/arg-parser": {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't know why, but this is once again un-hoisting the shell dependencies for some reason.

So normally I just do

npm i mongodb@latest @mongosh/logging@latest @mongosh/node-runtime-worker-thread@latest @mongosh/browser-repl@latest bson@latest @mongodb-js/devtools-connect@latest @mongodb-js/devtools-proxy-support@latest && git checkout -- package.json && npm i

now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! I didn't even notice that. Trying again..

const shellRef: ShellRef = useRef(null);
const canRenderShell = !!(enableShell && initialHistory && runtime);

// initialEvaluate will only be set on the first render
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: now that it's not tied to the React lifecycle of the browser-repl component, I'd maybe clarify that this is specifically a first render of the broser-repl Shell, not this component first render

@lerouxb lerouxb merged commit 9b1a79e into main Dec 17, 2024
5 of 6 checks passed
@lerouxb lerouxb deleted the shell-tab-switch branch December 17, 2024 09:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants