Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Repo info
Activity
    Rocco De Angelis
    @rdeangelis83
    Yep
    Colin Alworth
    @niloc132
    exactly
    if you're interested in good debugging (in SDM?), take a look at the patch i linked? someone else noticed that stack traces break in SDM or any "i loaded my gwt module from another origin", and i also noticed that promise events don't work either in this case, but only in chrome
    Rocco De Angelis
    @rdeangelis83
    @niloc132 I'm not 100% sure, but I thing the name property should be supported in must browser. But yes of course we could simply add both
    Colin Alworth
    @niloc132
    the calling code is picky, only accepts an expr statement... wonder if we could nest a comma expr or some garbage like that
    ha looks like we can, that's terrible
    addMethodDefinitionStatement only wants JsExprStmts to be added to it
    and only one, since outputDisplayName is expected to return a single thing
    maybe a second set of statements in the emitMethodImplementation's if check instead
    Rocco De Angelis
    @rdeangelis83
    I could try it out and open a PR but when is the next GWT release planed?
    Colin Alworth
    @niloc132
    i would be happy to push one forward if we get these changes in - i'll review and iterate with you if you can also take a look at my review above?
    would like to check back in with @tbroyer, last i heard he was close to a satisfactory way of starting groupId migration, but i never heard back on how that went
    Rocco De Angelis
    @rdeangelis83
    Okay I will build tomorrow a local version and give your changes a try and give you feedback ...
    Colin Alworth
    @niloc132

    @rdeangelis83

        private void emitMethodImplementation(JMethod method, JsNameRef functionNameRef,
            JsExprStmt methodDefinitionStatement) {
          addMethodDefinitionStatement(method, methodDefinitionStatement);
    
          if (shouldEmitDisplayNames()) {
            addMethodDefinitionStatement(method, outputDisplayName(functionNameRef, method));
            addMethodDefinitionStatement(method, outputFunctionNameProperty(functionNameRef, method));
          }
        }
    
        private void generatePrototypeDefinitionAlias(JMethod method, JsName alias) {
          JsName polyName = polymorphicNames.get(method);
          JsExpression bridge = JsUtils.createBridge(method, polyName, topScope);
          // Aliases are never property accessors.
          generatePrototypeAssignment(method, alias, bridge, JsMemberType.NONE);
        }
    
        private JsExprStmt outputDisplayName(JsNameRef function, JMethod method) {
          JsNameRef displayName = new JsNameRef(function.getSourceInfo(), "displayName");
          displayName.setQualifier(function);
          String displayStringName = getDisplayName(method);
          JsStringLiteral displayMethodName =
              new JsStringLiteral(function.getSourceInfo(), displayStringName);
          return createAssignment(displayName, displayMethodName).makeStmt();
        }
    
        private JsExprStmt outputFunctionNameProperty(JsNameRef function, JMethod method) {
          String displayStringName = getDisplayName(method);
          JsStringLiteral displayMethodName =
              new JsStringLiteral(function.getSourceInfo(), displayStringName);
          JsObjectLiteral descriptor = JsObjectLiteral.builder(function.getSourceInfo())
                  .add("value", displayMethodName)
                  .build();
          JsStringLiteral prop = new JsStringLiteral(function.getSourceInfo(), "name");
          return constructInvocation(function.getSourceInfo(), "Object.defineProperty", function, prop, descriptor).makeStmt();
        }

    try that out, i think it is close (plus or minus formatting)

    Thomas Broyer
    @tbroyer
    @niloc132 All my experiments and conclusions are at https://github.com/tbroyer/gwt-relocation-tests
    Colin Alworth
    @niloc132
    i see early conclusions, do you have final ones or advice?
    Thomas Broyer
    @tbroyer
    Apparently it needed more tests: https://groups.google.com/g/google-web-toolkit-contributors/c/L2RMqglOEXo/m/kCNHSaMeBwAJ (as noted in the project readme too)
    Colin Alworth
    @niloc132
    right, that's what i was intending to allude to, that we were close but not complete to a proposal
    needing more tests in gradle at least suggests someone proficient in gradle (i have done a lot of work since we last talked, but still consider it to be too complex to learn without already understanding it)
    i am not aware of any significant sbt usage of maven, and only minimal ivy usage
    Thomas Broyer
    @tbroyer
    Ha, that experiment-5 looked like those "more tests" actually; and prompted doing an experiment-6 deploying Gradle Module Metadata for GWT itself to confirm the early conclusions (so that gwt-user brings a "dependency constraint" on gwt-dev; similar to having the GWT BOM being transitively depended on). There would still be issues for Maven users not using the GWT BOM, that would have to be dealt with "manually" (using exclusions).
    But experiment-4 could be regarded as "enough" (particularly for a RC, for instance), or even experiment-3 (experiment-4 actually adds an o.g→c.g.g relocation for older versions, so can be added later)
    Colin Alworth
    @niloc132
    ok - so to briefly summarize (and possibly glossing over details), the "missing final post" to the thread could be assumed to say something like:
    • with proposal-5, the gradle experience is acceptable
    • maven requires the use of a bom to be sane, we will need to aggressively push this to any projects that updates either its gwt dependencies or any libraries that it uses, so we need to enlist library publishers to push the bom
    oh, why drop back to 3 or 4 if 5 is strictly better for the gradle cases?
    Thomas Broyer
    @tbroyer
    5 is actually the same as 4 as far as GWT is concerned (it adds Gradle Module Metadata to "third-party" libraries), and 4 is the same as 3 as far as deploying a new version of GWT is concerned (it adds relocations from o.g to c.g.g for older versions)
    Colin Alworth
    @niloc132
    right, that's what i'm trying to say, isnt 5 > 4 > 3? why suggest that we use 3 or 4 as "enough"?
    Thomas Broyer
    @tbroyer
    "6" would add *.module files to the GWT artifacts
    Well, 5 is "exercising" 4 with some external libraries; so as far as GWT is concerned, we would be "doing 4". And "doing 4" requires "doing 3" first (or next, actually).
    In terms of setting up the deployment for a new version, the aim is experiment-3 (it's identical experiment-4 and experiment-5). The additions of experiment-4 could be done separately to "make things better".
    And iterating from that, adding Gradle Module Metadata (similar to what's been proposed for Guava, except we're not interested in the variants here: https://github.com/google/guava/pull/3683/files) implies releasing a new version (it shouldn't stop us doing a first RC without them though)
    Colin Alworth
    @niloc132
    aside from a few extra lines in the bom for the extra relocations, is there a downside for 4? It seems like getting it out as a snapshot or RC with the ability to test mixes 2.7/2.8/2.9 dependencies/projects would be a benefit, rather than adding it later?
    i readily admit to not understanding the implications of 5
    Thomas Broyer
    @tbroyer
    🤔 which "few extra lines in the bom for the extra relocations"? I'm diffing 3 and 4 the only changes are the addition of org.gwtproject 2.9.0 version (and related change in maven-metadata)
    Colin Alworth
    @niloc132
    then i misunderstood the summary (tried to avoid reading the code but stick to the summary and conclusion, to not get caught up in the weeds beyond "does it solve the problems we anticipated")

    https://github.com/tbroyer/gwt-relocation-tests#experiment-4 says

    ...with an additional relocation from org.gwtproject to com.google.gwt for version 2.9.0

    which seems like "extra relocations", no?

    oh i see, not extra relocations, but extra releases
    Thomas Broyer
    @tbroyer
    Yes, those are "new" org.gwtproject:gwt:2.9.0, org.gwtproject:gwt-user:2.9.0 and org.gwtproject:gwt-dev:2.9.0 artifacts; that relocates to their com.google.gwt alter egos
    Colin Alworth
    @niloc132
    so, a library that supports 2.9.0 and isnt ready for 2.10.0 could still move groupid to make the future path simpler
    s/library/project/
    Thomas Broyer
    @tbroyer

    Yes, that's what I was trying to say in the early conclusions with:

    Using the org.gwtproject:gwt:2.9.0 BOM helps stay on GWT 2.9.0 without risking mixed com.google.gwt and org.gwtproject dependencies, by automatically downgrading the latter (to 2.9.0, which relocates to the former).

    Colin Alworth
    @niloc132
    thanks, sorry for my misunderstanding - i had interpreted it to say that if you used o.g:2.10 it would be able to point out c.g.g:2.9 and mark them as incompatible
    Thomas Broyer
    @tbroyer
    Note that such a project would use the org.gwtproject:gwt BOM, but the com.google.gwt:gwt-user and com.google.gwt:gwt-dev libraries
    Colin Alworth
    @niloc132
    right, and when it updates the o.g bom, would correctly move c.g.g -> o.g libraries
    Thomas Broyer
    @tbroyer
    yes (through relocations), but the project itself should then depend on org.gwtproject:gwt-user directly (rather than relying on the relocation)
    Colin Alworth
    @niloc132
    could i ask you to post such a summary on the mailing list, as a way to bring the topic up again, and we can make these changes in the repo in anticipation of a release? i also notice that you have a htmlunit/jetty/asm patch that you might want to merge, i have a draft, doesn't make sense for us to conflict on that (but i'll certainly assist in testing your commit if you'd like to proceed)
    Thomas Broyer
    @tbroyer
    So, first step: use org.gwtproject:gwt:2.9.0 BOM; then when updating: update to org.gwtproject:gwt:2.10.0 and change direct dependencies from com.google.gwt to org.gwtproject.
    Colin Alworth
    @niloc132
    migration to org.gwtproject, dealing with degraded debugging experience in chrome, updated dependencies seems like a good 2.10 to aim for - i also have that remotewebdriver runstyle that could be worth putting into gwt proper
    Thomas Broyer
    @tbroyer
    Well, I do not have an htmlunit/jetty/asm patch actually. If you're referring to the note in the https://github.com/tbroyer/gwt-relocation-tests#experiment-5, this project uses empty JARs, and I only modified the POMs 🤷
    Colin Alworth
    @niloc132
    oh got it
    in that case i'll pick that back up again, so that as we merge these other three sets of changes we can also get that done
    i only had it done locally, and it was for a now out-of-date jetty and htmlunit (and i didn't directly poke asm iirc, at least not for its own sake)