-
Notifications
You must be signed in to change notification settings - Fork 323
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 showLogs
#7568
Fix showLogs
#7568
Conversation
this.console[name] = console[name] | ||
console[name] = (...args: unknown[]) => { | ||
this.consume(name, args) | ||
if (shouldExecute) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't look right to me that we need such guards. If this code is being executed for the second time, we are already creating an extra new Router()
instance anyway, which I believe is incorrect. Usually top level code in JS modules have a guarantee of executing at most once, so it would be better to figure out why this rule is violated and fix that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Frizi the issue is that (currently) the frontend loads the application from the cloud, because the cloud may be running a different version of enso.runApp
...?
i don't remember the reasoning why i made the editor do that tbh. i can create an alternate PR that fixes this a different way
Closing as draft in favor of #7570. |
Fix for `showLogs` when opening a second cloud project. Alternative to #7568. This one avoids loading the cloud's `index.js.gz`, unless it is in the cloud - and in the cloud, it only loads the first `index.js.gz`.
Pull Request Description
Fix
showLogs
, by disabling it ifshowLogs
already existsImportant Notes
This will need to be pushed to cloud in order to work. Tested locally by temporary replacing the URL of the loaded script (normally cloud's
index.js.gz
) with the locally built one.Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:
The documentation has been updated, if necessary.Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.Scala,
Java,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.
Unit tests have been written where possible.If GUI codebase was changed, the GUI was tested when built using./run ide build
.