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

Enable HMR depending on debug mode #43601

Closed
wants to merge 2 commits into from
Closed

Conversation

onny
Copy link

@onny onny commented Feb 15, 2024

Summary

Currently Nextlouc needs to get patched to use Vue Devtools due to "hot module reloading" is restricted, see

This PR loosens security on debug mode and makes Vue Devtools usable

Checklist

Signed-off-by: Jonas Heinrich <[email protected]>
@onny onny marked this pull request as ready for review February 22, 2024 18:31
@onny
Copy link
Author

onny commented Feb 23, 2024

ready for review 😊 @skjnldsv @artonge

@@ -74,6 +74,17 @@ public function afterController($controller, $methodName, Response $response): R
$defaultPolicy->useJsNonce($this->csrfTokenManager->getToken()->getEncryptedValue());
}

// Loosen security presets in debug mode to enable development
// tools functionality
$debugging = \OC::$server->getConfig()->getSystemValue('debug', false);
Copy link
Member

@skjnldsv skjnldsv Feb 23, 2024

Choose a reason for hiding this comment

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

please use dependency injection in the construct :)
\OC::$server->getConfig() is deprecated

@skjnldsv skjnldsv added this to the Nextcloud 29 milestone Feb 23, 2024
This was referenced Mar 12, 2024
This was referenced Mar 20, 2024
@skjnldsv skjnldsv mentioned this pull request Mar 28, 2024
81 tasks
@skjnldsv skjnldsv modified the milestones: Nextcloud 29, Nextcloud 30 Mar 28, 2024
Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

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

I am strongly against this. Most developers have their dev setups in debug mode and never test without. So we'll likely run into features breaking in production because nobody notices CSP errors locally.

@onny
Copy link
Author

onny commented Jul 4, 2024

I am strongly against this. Most developers have their dev setups in debug mode and never test without. So we'll likely run into features breaking in production because nobody notices CSP errors locally.

would it be possible to introduce an extra system setting variable like "enable_hmr" which can be only enabled if debug mode is also enabled?

@ShGKme
Copy link
Contributor

ShGKme commented Jul 16, 2024

HMR requires connection to only one server on a specific host. A proper solution would be to fix only required restrictions for a specific dev server and not to disable CSP completely.

@ShGKme
Copy link
Contributor

ShGKme commented Jul 16, 2024

I'd personally prefer to have a solution in a separated app, not in the server.

@onny onny closed this Jul 16, 2024
@skjnldsv skjnldsv removed this from the Nextcloud 30 milestone Aug 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants