google / jsinterop-generator

Generates Java annotated with JsInterop from JavaScript extern sources
Apache License 2.0
78 stars 24 forks source link

Global scope is always assumed to be "window" #18

Open niloc132 opened 5 years ago

niloc132 commented 5 years ago

ModelHelper.createGlobalJavaType always assumes that window is the correct name to use:

  public static Type createGlobalJavaType(String packagePrefix, String globalScopeClassName) {
    Type type = new Type(NAMESPACE);
    type.setName(globalScopeClassName);
    type.setPackageName(packagePrefix);
    type.setNativeNamespace(GLOBAL_NAMESPACE);
    type.addAnnotation(
        builder()
            .type(JS_TYPE)
            .isNativeAttribute(true)
            .namespaceAttribute("")
            .nameAttribute("window")
            .build());
    return type;
  }

However, this is only correct in a case like DomGlobal, which only makes sense in a normal window context. For classes like Global, this should instead be something like self, so that it works both in windows and in workers (as well as other non-browser JS contexts).

My proposal would be to change this to invoke nameAttribute("self"), since that seems as though it should be valid in window and worker contexts alike (see [1], [2]), but perhaps there is a better, more flexible solution?

[1] https://developer.mozilla.org/en-US/docs/Web/API/Window/self [2] https://developer.mozilla.org/en-US/docs/Web/API/WorkerGlobalScope/self

In the mean time, objects in Global can't be used. Workarounds for others who find this:

In all of these cases, JsPackage.GLOBAL should be used as the namespace to avoid needing to reference window.

gkdn commented 5 years ago

goog.global is the replacement. self unfortunately doesn't work in node.js environments. We added goog.global to GWT earlier so we could update the code generation. (https://github.com/gwtproject/gwt/blob/4825d47eee7505346de372a159db55b18b006a2a).

vegegoku commented 5 years ago

the link seems to be broken.

tbroyer commented 5 years ago

https://github.com/gwtproject/gwt/commit/4825d47eee7505346de372a159db55b18b006a2a

realityforge commented 5 years ago

I would love to see a java constant added for this name such as JsType.GLOBAL with a little comment to indicate what it does. Although then we have both JsType.GLOBAL and JsType.GLOBAL. Maybe we should use the unfortunate naming convention adopted by TC39 ala JsType.GLOBAL_THIS. See https://github.com/tc39/proposal-global and https://github.com/tc39/proposal-global/blob/master/NAMING.md