Skip to content

Ignore property access when view is unloaded #202

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

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

tbragaf
Copy link
Collaborator

@tbragaf tbragaf commented May 16, 2025

No description provided.

@cssecautomation
Copy link

cssecautomation commented May 16, 2025

🎉 Snyk checks have passed. No issues have been found so far.

security/snyk check is complete. No issues have been found. (View Details)

license/snyk check is complete. No issues have been found. (View Details)

code/snyk check is complete. No issues have been found. (View Details)

@tbragaf tbragaf requested review from joaompneves and fmgracias May 16, 2025 09:06
@@ -64,7 +64,7 @@ export function loadPlugins(plugins: any[][], frameName: string): void {

view = tryGetView(frameName)!;
if (!view) {
return; // was probably unloaded
return; // view was probably unloaded
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

following the same strategy that we already have on load

@@ -10,7 +16,10 @@ export function createPropertiesProxy(rootElement: Element, objProperties: {}, n
} else {
proxy[key] = async function () {
const nativeObject = window[nativeObjName] || await bindNativeObject(nativeObjName);

if (!nativeObject && !isViewLoaded?.()) {
return; // view was probably unloaded
Copy link
Collaborator Author

@tbragaf tbragaf May 16, 2025

Choose a reason for hiding this comment

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

by returning undefined, we are moving errors to the views, which may then seem weird as in this.props.getSomething() will return undefined.
But the view should be protected from the possibility of props returning undefined.
If the props is void, then all good and we avoid the crash

Copy link
Collaborator Author

@tbragaf tbragaf May 16, 2025

Choose a reason for hiding this comment

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

just found out that the view is loaded and exists, but the nativeObject does not. I was trying to be less permissive, but i'm wondering now if I can just return if the nativeObject is not defined

@tbragaf tbragaf requested a review from heldergoncalves92 May 16, 2025 12:24
@tbragaf tbragaf marked this pull request as draft May 16, 2025 12:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants