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

Safely check the global appName and handle its inexistence #2028

Merged
merged 1 commit into from
Jun 14, 2021

Conversation

ChristophWurst
Copy link
Contributor

@ChristophWurst ChristophWurst commented Jun 10, 2021

Variable is set -> default is used, developers can overwrite the prop value
Variable is not set -> prop become required

Right now you can't use the AppContent without the global variable, because the code that sets the default is always evaluated and will throw a ReferenceError before the developer can even specify their own value. Ref nextcloud/mail#5165 (comment).

Then we can avoid hacks like nextcloud/spreed@c662744

The code is untested because neither @GretaD nor I can build this library at the moment.

@ChristophWurst ChristophWurst added bug Something isn't working 3. to review Waiting for reviews labels Jun 10, 2021
@ChristophWurst ChristophWurst added this to the 4.0.1 milestone Jun 10, 2021
@ChristophWurst ChristophWurst self-assigned this Jun 10, 2021
@ChristophWurst ChristophWurst force-pushed the fix/app-name-global-save-checking branch 2 times, most recently from 64a2f4c to c59679e Compare June 10, 2021 16:07
@PVince81

This comment has been minimized.

@ChristophWurst

This comment has been minimized.

@skjnldsv

This comment has been minimized.

@raimund-schluessler

This comment has been minimized.

@ChristophWurst

This comment has been minimized.

@skjnldsv

This comment has been minimized.

Copy link
Contributor Author

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

+1

Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
@skjnldsv skjnldsv force-pushed the fix/app-name-global-save-checking branch from fbf756f to 5688b2a Compare June 11, 2021 16:51
@ChristophWurst ChristophWurst merged commit 1320ded into master Jun 14, 2021
@ChristophWurst ChristophWurst deleted the fix/app-name-global-save-checking branch June 14, 2021 07:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants