You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@royale.apache.org by GitBox <gi...@apache.org> on 2020/01/02 21:59:18 UTC
[GitHub] [royale-asjs] Harbs opened a new issue #649: Externs warning
Harbs opened a new issue #649: Externs warning
URL: https://github.com/apache/royale-asjs/issues/649
When compiling HelloWorld, we get the following warning:
````
[mxmlc] Jan 02, 2020 10:54:05 PM com.google.javascript.jscomp.LoggerErrorManager println
[mxmlc] WARNING: externs/dialogPolyfill.js:19: WARNING - accessing name dialogPolyfill in externs has no effect. Perhaps you forgot to add a var keyword?
[mxmlc] dialogPolyfill = function() {
[mxmlc] ^^^^^^^^^^^^^^
[mxmlc]
[mxmlc] Jan 02, 2020 10:54:05 PM com.google.javascript.jscomp.LoggerErrorManager println
[mxmlc] WARNING: externs/dialogPolyfill.js:19: WARNING - variable dialogPolyfill is undeclared
[mxmlc] dialogPolyfill = function() {
[mxmlc] ^^^^^^^^^^^^^^
[mxmlc]
[mxmlc] Jan 02, 2020 10:54:05 PM com.google.javascript.jscomp.LoggerErrorManager println
[mxmlc] WARNING: externs/dialogPolyfill.js:27: WARNING - name dialogPolyfill is not defined in the externs.
[mxmlc] dialogPolyfill.registerDialog = function(dialog) {
````
This appears to be from the following in Jewel:
````
/**
* Generated by Apache Royale Compiler from dialogPolyfill.as
* dialogPolyfill
*
* @fileoverview
* @externs
*
* @suppress {checkTypes|accessControls}
*/
/**
* <inject_html>
* <script src="https://cdnjs.cloudflare.com/ajax/libs/dialog-polyfill/0.4.9/dialog-polyfill.min.js"></script>
* <link rel="stylesheet" type="text/css" href="https://cdnjs.cloudflare.com/ajax/libs/dialog-polyfill/0.4.9/dialog-polyfill.min.css">
* </inject_html>
* @constructor
*/
dialogPolyfill = function() {
};
/**
* @export
* @param {Element} dialog
*/
dialogPolyfill.registerDialog = function(dialog) {
};
````
which is generated by this:
````
package
{
/**
* @externs
*/
COMPILE::JS
public class dialogPolyfill
{
/**
* <inject_html>
* <script src="https://cdnjs.cloudflare.com/ajax/libs/dialog-polyfill/0.4.9/dialog-polyfill.min.js"></script>
* <link rel="stylesheet" type="text/css" href="https://cdnjs.cloudflare.com/ajax/libs/dialog-polyfill/0.4.9/dialog-polyfill.min.css">
* </inject_html>
*/
public function dialogPolyfill(){}
public static function registerDialog(dialog:Element):void {}
}
}
````
I'm not sure why that's the result or how to fix it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [royale-asjs] greg-dove commented on issue #649: Externs warning
Posted by GitBox <gi...@apache.org>.
greg-dove commented on issue #649: Externs warning
URL: https://github.com/apache/royale-asjs/issues/649#issuecomment-570412928
I did not look into this deeply, but I have seen the same on other occasions as well.
But just to add something for now: I can see this warning for HelloWorld in ant build, but not with maven build. So maybe it is to do with scope of dependencies specified or something like this.... perhaps ant build includes everything (including Jewel) and maven does not include Jewel dependency...?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [royale-asjs] carlosrovira commented on issue #649: Externs warning
Posted by GitBox <gi...@apache.org>.
carlosrovira commented on issue #649: Externs warning
URL: https://github.com/apache/royale-asjs/issues/649#issuecomment-570395755
Hi @Harbs, interesting. I understand that Hello World example is not using Jewel, right? So how is possible that is participating in the compilation?
In the other hand, I take dialog for two reasons:
1.- To try polyfill: I think is important to have some polyfill in real cases, and this one seemed to me something needed and as well a good way to test it. Problems like this one you raised are showing thanks to have something like this in place
2.- dialog is from what I read at the moment (when working on MDL) the way to go. Didn't check back again, and that's around 2 years ago, maybe plans in browsers lower from that time. Don't know, but if dialog is still a tag for the future, I think we should continue using it.
For me the problem is more about why we are getting that compilation issue, and not about the dialog tag suitability.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [royale-asjs] Harbs commented on issue #649: Externs warning
Posted by GitBox <gi...@apache.org>.
Harbs commented on issue #649: Externs warning
URL: https://github.com/apache/royale-asjs/issues/649#issuecomment-570531152
> Collateral to this, seems to me that ANT should not use all SWCs in config for all proyects. Apps should use the ones they use. Maybe a minor problem, but don't seems to me something to left in the long term.
To me this is a feature. Any piece of the framework is always available to use without configuring anything. That goes for Ant as well as asconfigc with the default Royale framework.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [royale-asjs] carlosrovira commented on issue #649: Externs warning
Posted by GitBox <gi...@apache.org>.
carlosrovira commented on issue #649: Externs warning
URL: https://github.com/apache/royale-asjs/issues/649#issuecomment-570549842
> To me this is a feature. Any piece of the framework is always available to use without configuring anything. That goes for Ant as well as asconfigc with the default Royale framework.
I'll not enter into that, but one of the bad side effects is what we are seeing here (far beyond the bug with externs), you get an unwanted and un needed process of a library and its classes and definitions.
I'll definitely does not see that as a feature, and will not like to have it in Maven builds, but anyway, people using ANT can search for other needs see it as a feature, but I suppose that will be at a cost of compiling time processing of all of that definitions by the compiler. Maybe not many time, but since we're looking this days for optimizations, not seems to me something that goes in that way.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [royale-asjs] Harbs commented on issue #649: Externs warning
Posted by GitBox <gi...@apache.org>.
Harbs commented on issue #649: Externs warning
URL: https://github.com/apache/royale-asjs/issues/649#issuecomment-571189760
Related: I'm getting the polyfill in an application which shouldn't require it.
Here is an example:
https://github.com/unhurdle/spectrum-royale/tree/include-css/SpectrumBrowser
When compiled, I get these lines:
````
<link rel="stylesheet" type="text/css" href="SpectrumBrowser.min.css">
<script src="./assets/icons/loadIcons.js"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/dialog-polyfill/0.4.9/dialog-polyfill.min.js"></script>
<link rel="stylesheet" type="text/css" href="https://cdnjs.cloudflare.com/ajax/libs/dialog-polyfill/0.4.9/dialog-polyfill.min.css">
````
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [royale-asjs] aharui commented on issue #649: Externs warning
Posted by GitBox <gi...@apache.org>.
aharui commented on issue #649: Externs warning
URL: https://github.com/apache/royale-asjs/issues/649#issuecomment-571223848
@joshtynjala is correct that the externs is probably not output correctly
@Harbs is correct that the externs shouldn't be in some apps.
IMO this issue should be moved to royale-compiler
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [royale-asjs] Harbs commented on issue #649: Externs warning
Posted by GitBox <gi...@apache.org>.
Harbs commented on issue #649: Externs warning
URL: https://github.com/apache/royale-asjs/issues/649#issuecomment-570376636
Related (although I think the underlying issue with `@externs` is something that needs to be addressed):
Why is a `<dialog>` and a polyfill being used for Jewel Dialogs? It's quite possible (easy even) to use normal divs to create beautiful dialogs. You can use Spectrum as a reference: https://opensource.adobe.com/spectrum-css/components/dialog/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [royale-asjs] piotrzarzycki21 commented on issue #649: Externs
warning
Posted by GitBox <gi...@apache.org>.
piotrzarzycki21 commented on issue #649: Externs warning
URL: https://github.com/apache/royale-asjs/issues/649#issuecomment-570552054
> I'll definitely does not see that as a feature, and will not like to have it in Maven builds, but anyway, people using ANT can search for other needs see it as a feature, but I suppose that will be at a cost of compiling time processing of all of that definitions by the compiler. Maybe not many time, but since we're looking this days for optimizations, not seems to me something that goes in that way.
If in Maven distribution build you will not add all of that - how IDE will know that something is available ? This is why ANT has everything in the config - At least this is how I understand that.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [royale-asjs] carlosrovira commented on issue #649: Externs warning
Posted by GitBox <gi...@apache.org>.
carlosrovira commented on issue #649: Externs warning
URL: https://github.com/apache/royale-asjs/issues/649#issuecomment-570529230
I think @joshtynjala knows better about @externs and maybe could bring some light on this.
Collateral to this, seems to me that ANT should not use all SWCs in config for all proyects. Apps should use the ones they use. Maybe a minor problem, but don't seems to me something to left in the long term.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [royale-asjs] Harbs commented on issue #649: Externs warning
Posted by GitBox <gi...@apache.org>.
Harbs commented on issue #649: Externs warning
URL: https://github.com/apache/royale-asjs/issues/649#issuecomment-570564034
> I'll not enter into that, but one of the bad side effects is what we are seeing here (far beyond the bug with externs), you get an unwanted and un needed process of a library and its classes and definitions.
Huh? It does not add anything to the app.
Does it add to the compile time? I don't know. I guess theoretically there is some extra overhead, but I don't know if it's enough to be measurable. Before declaring something a problem, I think we should have some data.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [royale-asjs] carlosrovira commented on issue #649: Externs warning
Posted by GitBox <gi...@apache.org>.
carlosrovira commented on issue #649: Externs warning
URL: https://github.com/apache/royale-asjs/issues/649#issuecomment-570555646
> If in Maven distribution build you will not add all of that - how IDE will know that something is available ?
That's for me a problem for IDEs. I remember to talk with Josh about it, and determined it could be good to solve some day.
Lets say I'm using Jewel in an app, and don't want to have Basic, MDL, MX, Spark, CreateJS, JQuery,...(lots of not required libs).
Right now, if you write `Bu` and make code intelligence show to add the proper `Button` class you'll find with lots of button classes available, but you know that you just want the Jewel one. Then you need to read each one until you find the Jewel Button and hit enter.
I think that's bad user experience for a developer.
I expect to have a royale config someday for Jewel that removes CreateJS, MDL, MX and SPARK (when MXRPC will be separated from visual components). I'll not be able to do that for BASIC, since is mixed with the rest of foundation classes, so I'll have at least 2 Button options, but will remove lots of unwanted options I'll never use.
And moreover, will never have the mistake of using a class in a library by mistake that could pull other unwanted dependencies. @aharui talk about this just few days ago in other thread about the danger of this (I think it was in the thread about abstraction of platform specifics patterns).
I don't see any benefits on having lots of libraries we don't use to be processed by build tools or IDEs, nothing but confuse the user, make the processing times longer and make the user (many of them not as experts as others) adding classes with similar definitions to their apps and pull other unwanted definitions in doing that.
just my 2.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [royale-asjs] joshtynjala commented on issue #649: Externs warning
Posted by GitBox <gi...@apache.org>.
joshtynjala commented on issue #649: Externs warning
URL: https://github.com/apache/royale-asjs/issues/649#issuecomment-571175819
> When compiling HelloWorld, we get the following warning:
> WARNING: externs/dialogPolyfill.js:19: WARNING - accessing name dialogPolyfill in externs has no effect. Perhaps you forgot to add a var keyword?
As best I can tell, there are no issues from the way that we generated JavaScript code. However, I agree that this warning is annoying.
I think that the fix is to do exactly as it suggests and add a `var` keyword before the name of the variable, like this:
```
var dialogPolyfill = function() {
};
```
Technically, it's not required for valid JavaScript, since it creates a global variable either way, but it seems that Closure compiler prefers being explicit with the `var` keyword. Presumably, Closure compiler would not change its minifying behavior if we added `var`, but I'll check before I make the change.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [royale-asjs] carlosrovira commented on issue #649: Externs warning
Posted by GitBox <gi...@apache.org>.
carlosrovira commented on issue #649: Externs warning
URL: https://github.com/apache/royale-asjs/issues/649#issuecomment-570579088
@piotrzarzycki21 maybe @joshtynjala could remember where we talk about it, I don't know if it was in his issues repo or privately (sure was one of that).
@Harbs I think is both: an overhead problem and a user experience problem, along with the possible cases of users using classes that they really don't want (I'm sure we all here has some time hit ctrl enter in an IDE and the import you get is not what you want, then you notice and change for the right one).
Being said that, we can think is not a "big" problem, and we can avoid for now, but don't think is good to refer it as a feature, since not begin explicit with the libraries you want to use, can have all those 3 side effects, and don't think any of the 3 are good.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [royale-asjs] piotrzarzycki21 commented on issue #649: Externs
warning
Posted by GitBox <gi...@apache.org>.
piotrzarzycki21 commented on issue #649: Externs warning
URL: https://github.com/apache/royale-asjs/issues/649#issuecomment-570560828
> That's for me a problem for IDEs. I remember to talk with Josh about it, and determined it could be good to solve some day.
Could you point to discussion about that ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [royale-asjs] aharui commented on issue #649: Externs warning
Posted by GitBox <gi...@apache.org>.
aharui commented on issue #649: Externs warning
URL: https://github.com/apache/royale-asjs/issues/649#issuecomment-570473071
The Ant build uses a config file that brings all SWCs except MXRoyale into the library-path. For Maven you specify exactly which SWCs you need.
Could be that @externs shouldn't be processed unless found as a dependency. I don't recall from memory whether that's currently true or not. Someone will have to dig into the compiler.
And then if the extern is not processed, then the inject-html shouldn't either.
There is a separate issue that it appears that the extern is incorrectly generated, causing the warning in the first place.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services