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

bug(api): withGlobalTauri injection order is wrong and can't access window #2289

Closed
lemarier opened this issue Jul 23, 2021 · 8 comments
Closed

Comments

@lemarier
Copy link
Member

Describe the bug

When we enable withGlobalTauri we got;

TypeError: undefined is not an object (evaluating 'window.__TAURI__.__currentWindow.label')

To Reproduce

Steps to reproduce the behavior:
cargo run --bin bench_helloworld

Expected behavior

Should work without errors

Screenshots

N/A

Additional context

Introduced by #2285

@lemarier
Copy link
Member Author

(function () {{
const __TAURI_INVOKE_KEY__ = {key};
{bundle_script}
}})()

@lemarier
Copy link
Member Author

lemarier commented Jul 23, 2021

@amrbashir I think we should deprecate the use of appWindow and use getCurrent() instead, otherwise it'll be too late if we wait the stable release.

@amrbashir
Copy link
Member

I agree, removing it is the best approach but that would be a huge breaking change, so many people use it and we use it alot through the documentation but lets see what @nothingismagick and @lucasfernog think.

@nothingismagick
Copy link
Sponsor Member

I would like to hear @chippers opinion here, and while it is true this is a breaking change, we are in beta. What we cannot afford to do, is to let any such issues slip through before the audit begins. After the audit (and the subsequent stable release), breaking changes of any kind will force a Major Version bump.

@chippers
Copy link
Member

I think that supporting appWindow to prevent a large breaking change is probably worthwhile since the additional code to do so is not a very large burden as far as I can see. Especially since it's already so prevalent throughout all our documentation. I agree that having getCurrent() being the only way of doing things is my preference, but unfortunately it's not.

Here's a diff from dev that allows it to work with and without withGlobalTauri.

diff --git a/core/tauri/src/manager.rs b/core/tauri/src/manager.rs
index 5d681010..8ead3781 100644
--- a/core/tauri/src/manager.rs
+++ b/core/tauri/src/manager.rs
@@ -234,16 +234,19 @@ impl<R: Runtime> WindowManager<R> {
       .expect("poisoned plugin store")
       .initialization_script();
 
+    // convert the label into a formatted js string
+    let label = serde_json::to_string(&label)?;
+
     let mut webview_attributes = pending.webview_attributes;
     webview_attributes = webview_attributes
-      .initialization_script(&self.initialization_script(&plugin_init, is_init_global))
+      .initialization_script(&self.initialization_script(&plugin_init, is_init_global, &label))
       .initialization_script(&format!(
         r#"
           window.__TAURI__.__windows = {window_labels_array}.map(function (label) {{ return {{ label: label }} }});
           window.__TAURI__.__currentWindow = {{ label: {current_window_label} }}
         "#,
         window_labels_array = serde_json::to_string(pending_labels)?,
-        current_window_label = serde_json::to_string(&label)?,
+        current_window_label = label
       ));
 
     #[cfg(dev)]
@@ -410,11 +413,13 @@ impl<R: Runtime> WindowManager<R> {
     &self,
     plugin_initialization_script: &str,
     with_global_tauri: bool,
+    label: &str,
   ) -> String {
     let key = self.generate_invoke_key();
     format!(
       r#"
       (function () {{
+        const __TAURI_CURRENT_WINDOW__ = {label};
         const __TAURI_INVOKE_KEY__ = {key};
         {bundle_script}
       }})()
@@ -430,6 +435,7 @@ impl<R: Runtime> WindowManager<R> {
       {plugin_initialization_script}
     "#,
       key = key,
+      label = label,
       core_script = include_str!("../scripts/core.js").replace("_KEY_VALUE_", &key.to_string()),
       bundle_script = if with_global_tauri {
         include_str!("../scripts/bundle.js")
diff --git a/tooling/api/src/window.ts b/tooling/api/src/window.ts
index 8f65d675..28d9d652 100644
--- a/tooling/api/src/window.ts
+++ b/tooling/api/src/window.ts
@@ -173,6 +173,10 @@ declare global {
   }
 }
 
+// this bundle can be invoked by withGlobalTauri which declares the following in-scope variable
+// eslint-disable-next-line @typescript-eslint/naming-convention
+declare const __TAURI_CURRENT_WINDOW__: string | undefined
+
 /** Attention type to request on a window. */
 enum UserAttentionType {
   /**
@@ -195,7 +199,15 @@ enum UserAttentionType {
  * @return The current WebviewWindow.
  */
 function getCurrent(): WebviewWindow {
-  return new WebviewWindow(window.__TAURI__.__currentWindow.label, {
+  // __TAURI_CURRENT_WINDOW__ is only defined for global bundles
+  let label: string
+  if (typeof __TAURI_CURRENT_WINDOW__ === 'undefined') {
+    label = window.__TAURI__.__currentWindow.label
+  } else {
+    label = __TAURI_CURRENT_WINDOW__
+  }
+
+  return new WebviewWindow(label, {
     // @ts-expect-error
     skip: true
   })
@@ -1108,10 +1120,7 @@ class WebviewWindow extends WindowManager {
 }
 
 /** The WebviewWindow for the current window. */
-const appWindow = new WebviewWindow(window.__TAURI__.__currentWindow.label, {
-  // @ts-expect-error
-  skip: true
-})
+const appWindow = getCurrent()
 
 /** Configuration for the window to create. */
 interface WindowOptions {

@lemarier
Copy link
Member Author

@chippers good points! I totally agree with you there, I didn't realize it was throughout all our documentation.
Do you want to open a PR?

@lucasfernog
Copy link
Member

This is my bad, I thought I pushed the fix for it but seems like I missed it. I wrote a simpler solution when I pushed that change, it was just allowing WebviewWindow label to be null.

@lucasfernog
Copy link
Member

I can make that PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants