Re: [royale-compiler] branch develop updated: PackageHeaderEmitter: output goog.require() for externs because release builds fail without it

2019-07-09 Thread Carlos Rovira 
Hi,

definitely, needing to add a new library will add overhead, and will turn
things more complex and cumbersome without a real need of organization.
IMO, we use to break into project libraries for better organization.

In this case concrete case (Jewel). both extern classes seems to be more
generic and could be used by people not using Jewel so I can move to other
library without problem.

Let me know if you think I should do that and in that case what name would
be the best for that project/library/swc.
If you want the opposite because you want to use it as a bank test to test
a new config implementation to solve this, let me know and I'll left as is
until we decide to do a separation.

thanks



El mar., 9 jul. 2019 a las 19:02, Alex Harui ()
escribió:

> Well, we should see how hard it would be to allow mixing @externs and
> regular AS files.  I think you are right that we are probably just finding
> issues with source-externs in libraries instead of the main app.
>
> That said, there may be a difference between supporting some third-party
> library vs supporting browser-specific extensions to the built-in classes.
> So additional work may be needed there as well.
>
> But we do want to make it easy for someone to wrap up a third-party
> library for use in Royale.  Right now they may have to create both a
> typedefs SWC and a framework SWC, so finding a way to combine the two may
> be useful.
>
> My 2 cents,
> -Alex
>
> On 7/9/19, 9:55 AM, "Josh Tynjala"  wrote:
>
> It seems that Jewel is being compiled with a couple of ActionScript
> classes
> marked with @externs. Perhaps that's the problem. Maybe those classes
> should be compiled into a separate SWC. Maybe the compiler should not
> allow
> classes with @externs and classes without it in the same SWC.
>
> --
> Josh Tynjala
> Bowler Hat LLC <
> https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbowlerhat.devdata=02%7C01%7Caharui%40adobe.com%7Cd17acb5cd03e4673fed308d7048e3183%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C1%7C636982881074522050sdata=ThduFZ8JuYyOBxzn1R7nW5nKwr7kzRkKn030MwFuQmc%3Dreserved=0
> >
>
>
> On Tue, Jul 9, 2019 at 9:35 AM Alex Harui 
> wrote:
>
> > I can't tell from this thread what an "extern" is, but I believe at
> least
> > in js-debug, if you goog.require something that doesn't have a
> goog.provide
> > in it, there can be other problems.  So maybe it was already broken,
> but
> > this does not seem like a positive change.  I expect other things to
> > break.  I'm surprised the GoogDepsWriter didn't output warnings or
> errors.
> >
> > Most Externs are in royale-typedefs and their SWCs should be on the
> > -external-library-path.  In theory, their .js files get handed to
> Google
> > Closure Compiler as externs not as regular js files.
> >
> > Source externs are supposed to do the same thing.
> >
> > IIRC, if you use a typedefs SWC to compile a framework SWC, then the
> > typedefs SWC must be in the external-library-path for the
> application that
> > uses the framework SWC.
> >
> > Thanks,
> > -Alex
> >
> > On 7/9/19, 9:11 AM, "Josh Tynjala" 
> wrote:
> >
> > Without a goog.require(), the .js file for the extern does not
> seem to
> > be
> > copied to the output folder, and Closure renames its APIs.
> >
> > How the compiler behaved *before* my change:
> >
> > * If I reference the extern in a library, the compiler did not
> add a
> > goog.require() for it.
> > * If I reference the extern in an application, *the compiler
> added a
> > goog.require() for it.*
> >
> > How it works after my change:
> >
> > * If I reference the extern in a library, the compiler adds a
> > goog.require() for it.
> > * If I reference the extern in an application, the compiler adds
> a
> > goog.require() for it.
> >
> > I made the compiler's behavior more consistent because the it was
> > already
> > adding goog.require() for externs in applications. If that's
> wrong, it
> > was
> > already wrong before I made any changes.
> >
> > --
> > Josh Tynjala
> > Bowler Hat LLC <
> >
> https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbowlerhat.devdata=02%7C01%7Caharui%40adobe.com%7Cd17acb5cd03e4673fed308d7048e3183%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C1%7C636982881074522050sdata=ThduFZ8JuYyOBxzn1R7nW5nKwr7kzRkKn030MwFuQmc%3Dreserved=0
> > >
> >
> >
> > On Mon, Jul 8, 2019 at 11:26 PM Alex Harui
> 
> > wrote:
> >
> > > This doesn't seem right to me.  Externs should be representing
> JS
> > files
> > > that don't have goog.provide in them.
> > >
> > > -Alex
> > >
> > > On 7/8/19, 1:40 PM, "joshtynj...@apache.org" <
> joshtynj...@apache.org
> > >
>

Re: [royale-compiler] branch develop updated: PackageHeaderEmitter: output goog.require() for externs because release builds fail without it

2019-07-09 Thread Alex Harui 
Well, we should see how hard it would be to allow mixing @externs and regular 
AS files.  I think you are right that we are probably just finding issues with 
source-externs in libraries instead of the main app.

That said, there may be a difference between supporting some third-party 
library vs supporting browser-specific extensions to the built-in classes.  So 
additional work may be needed there as well.

But we do want to make it easy for someone to wrap up a third-party library for 
use in Royale.  Right now they may have to create both a typedefs SWC and a 
framework SWC, so finding a way to combine the two may be useful.

My 2 cents,
-Alex

On 7/9/19, 9:55 AM, "Josh Tynjala"  wrote:

It seems that Jewel is being compiled with a couple of ActionScript classes
marked with @externs. Perhaps that's the problem. Maybe those classes
should be compiled into a separate SWC. Maybe the compiler should not allow
classes with @externs and classes without it in the same SWC.

--
Josh Tynjala
Bowler Hat LLC 



On Tue, Jul 9, 2019 at 9:35 AM Alex Harui  wrote:

> I can't tell from this thread what an "extern" is, but I believe at least
> in js-debug, if you goog.require something that doesn't have a 
goog.provide
> in it, there can be other problems.  So maybe it was already broken, but
> this does not seem like a positive change.  I expect other things to
> break.  I'm surprised the GoogDepsWriter didn't output warnings or errors.
>
> Most Externs are in royale-typedefs and their SWCs should be on the
> -external-library-path.  In theory, their .js files get handed to Google
> Closure Compiler as externs not as regular js files.
>
> Source externs are supposed to do the same thing.
>
> IIRC, if you use a typedefs SWC to compile a framework SWC, then the
> typedefs SWC must be in the external-library-path for the application that
> uses the framework SWC.
>
> Thanks,
> -Alex
>
> On 7/9/19, 9:11 AM, "Josh Tynjala"  wrote:
>
> Without a goog.require(), the .js file for the extern does not seem to
> be
> copied to the output folder, and Closure renames its APIs.
>
> How the compiler behaved *before* my change:
>
> * If I reference the extern in a library, the compiler did not add a
> goog.require() for it.
> * If I reference the extern in an application, *the compiler added a
> goog.require() for it.*
>
> How it works after my change:
>
> * If I reference the extern in a library, the compiler adds a
> goog.require() for it.
> * If I reference the extern in an application, the compiler adds a
> goog.require() for it.
>
> I made the compiler's behavior more consistent because the it was
> already
> adding goog.require() for externs in applications. If that's wrong, it
> was
> already wrong before I made any changes.
>
> --
> Josh Tynjala
> Bowler Hat LLC <
> 
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbowlerhat.devdata=02%7C01%7Caharui%40adobe.com%7Cd17acb5cd03e4673fed308d7048e3183%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C1%7C636982881074522050sdata=ThduFZ8JuYyOBxzn1R7nW5nKwr7kzRkKn030MwFuQmc%3Dreserved=0
> >
>
>
> On Mon, Jul 8, 2019 at 11:26 PM Alex Harui 
> wrote:
>
> > This doesn't seem right to me.  Externs should be representing JS
> files
> > that don't have goog.provide in them.
> >
> > -Alex
> >
> > On 7/8/19, 1:40 PM, "joshtynj...@apache.org"  >
> > wrote:
> >
> > This is an automated email from the ASF dual-hosted git
> repository.
> >
> > joshtynjala pushed a commit to branch develop
> > in repository
> >
> 
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitbox.apache.org%2Frepos%2Fasf%2Froyale-compiler.gitdata=02%7C01%7Caharui%40adobe.com%7Cd17acb5cd03e4673fed308d7048e3183%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C1%7C636982881074522050sdata=d%2FaFSs74bDyCZK8SX5y6agchBw6ZLCOauCMwSPQPaZM%3Dreserved=0
> >
> >
> > The following commit(s) were added to refs/heads/develop by this
> push:
> >  new 942a423  PackageHeaderEmitter: output goog.require() 
for
> > externs because release builds fail without it
> > 942a423 is described below
> >
> > commit 942a4235758020239eca707a5e5c73e94a63df8c
> > Author: Josh Tynjala 
> >

Re: [royale-compiler] branch develop updated: PackageHeaderEmitter: output goog.require() for externs because release builds fail without it

2019-07-09 Thread Josh Tynjala 
It seems that Jewel is being compiled with a couple of ActionScript classes
marked with @externs. Perhaps that's the problem. Maybe those classes
should be compiled into a separate SWC. Maybe the compiler should not allow
classes with @externs and classes without it in the same SWC.

--
Josh Tynjala
Bowler Hat LLC 


On Tue, Jul 9, 2019 at 9:35 AM Alex Harui  wrote:

> I can't tell from this thread what an "extern" is, but I believe at least
> in js-debug, if you goog.require something that doesn't have a goog.provide
> in it, there can be other problems.  So maybe it was already broken, but
> this does not seem like a positive change.  I expect other things to
> break.  I'm surprised the GoogDepsWriter didn't output warnings or errors.
>
> Most Externs are in royale-typedefs and their SWCs should be on the
> -external-library-path.  In theory, their .js files get handed to Google
> Closure Compiler as externs not as regular js files.
>
> Source externs are supposed to do the same thing.
>
> IIRC, if you use a typedefs SWC to compile a framework SWC, then the
> typedefs SWC must be in the external-library-path for the application that
> uses the framework SWC.
>
> Thanks,
> -Alex
>
> On 7/9/19, 9:11 AM, "Josh Tynjala"  wrote:
>
> Without a goog.require(), the .js file for the extern does not seem to
> be
> copied to the output folder, and Closure renames its APIs.
>
> How the compiler behaved *before* my change:
>
> * If I reference the extern in a library, the compiler did not add a
> goog.require() for it.
> * If I reference the extern in an application, *the compiler added a
> goog.require() for it.*
>
> How it works after my change:
>
> * If I reference the extern in a library, the compiler adds a
> goog.require() for it.
> * If I reference the extern in an application, the compiler adds a
> goog.require() for it.
>
> I made the compiler's behavior more consistent because the it was
> already
> adding goog.require() for externs in applications. If that's wrong, it
> was
> already wrong before I made any changes.
>
> --
> Josh Tynjala
> Bowler Hat LLC <
> https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbowlerhat.devdata=02%7C01%7Caharui%40adobe.com%7Cc589f5d7d32c425579c208d70488262a%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C1%7C636982855118961103sdata=B4z9PGs%2FVKFu7ZOPupm1UTRVxdw38MEx2yehIFPNKmc%3Dreserved=0
> >
>
>
> On Mon, Jul 8, 2019 at 11:26 PM Alex Harui 
> wrote:
>
> > This doesn't seem right to me.  Externs should be representing JS
> files
> > that don't have goog.provide in them.
> >
> > -Alex
> >
> > On 7/8/19, 1:40 PM, "joshtynj...@apache.org"  >
> > wrote:
> >
> > This is an automated email from the ASF dual-hosted git
> repository.
> >
> > joshtynjala pushed a commit to branch develop
> > in repository
> >
> https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitbox.apache.org%2Frepos%2Fasf%2Froyale-compiler.gitdata=02%7C01%7Caharui%40adobe.com%7Cc589f5d7d32c425579c208d70488262a%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C1%7C636982855118961103sdata=C4JjqtBaAcFyhUCORG7jgDP3X0bthRGexYZVS%2B1A29Q%3Dreserved=0
> >
> >
> > The following commit(s) were added to refs/heads/develop by this
> push:
> >  new 942a423  PackageHeaderEmitter: output goog.require() for
> > externs because release builds fail without it
> > 942a423 is described below
> >
> > commit 942a4235758020239eca707a5e5c73e94a63df8c
> > Author: Josh Tynjala 
> > AuthorDate: Mon Jul 8 13:40:05 2019 -0700
> >
> > PackageHeaderEmitter: output goog.require() for externs
> because
> > release builds fail without it
> > ---
> >
> .../royale/compiler/internal/codegen/js/jx/PackageHeaderEmitter.java
> >  | 3 ---
> >  1 file changed, 3 deletions(-)
> >
> > diff --git
> >
> a/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/jx/PackageHeaderEmitter.java
> >
> b/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/jx/PackageHeaderEmitter.java
> > index 7c8b9ac..57cfc76 100644
> > ---
> >
> a/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/jx/PackageHeaderEmitter.java
> > +++
> >
> b/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/jx/PackageHeaderEmitter.java
> > @@ -380,9 +380,6 @@ public class PackageHeaderEmitter extends
> > JSSubEmitter implements
> >
> >  if (imp.equals(cname))
> >  continue;
> > -
> > -if (project.sourceExterns.contains(imp))
> > -   continue;
> >
> >  if (NativeUtils.isNative(imp))
> >  {
> >
> >

Re: [royale-compiler] branch develop updated: PackageHeaderEmitter: output goog.require() for externs because release builds fail without it

2019-07-09 Thread Alex Harui 
I can't tell from this thread what an "extern" is, but I believe at least in 
js-debug, if you goog.require something that doesn't have a goog.provide in it, 
there can be other problems.  So maybe it was already broken, but this does not 
seem like a positive change.  I expect other things to break.  I'm surprised 
the GoogDepsWriter didn't output warnings or errors.

Most Externs are in royale-typedefs and their SWCs should be on the 
-external-library-path.  In theory, their .js files get handed to Google 
Closure Compiler as externs not as regular js files.

Source externs are supposed to do the same thing.

IIRC, if you use a typedefs SWC to compile a framework SWC, then the typedefs 
SWC must be in the external-library-path for the application that uses the 
framework SWC.

Thanks,
-Alex

On 7/9/19, 9:11 AM, "Josh Tynjala"  wrote:

Without a goog.require(), the .js file for the extern does not seem to be
copied to the output folder, and Closure renames its APIs.

How the compiler behaved *before* my change:

* If I reference the extern in a library, the compiler did not add a
goog.require() for it.
* If I reference the extern in an application, *the compiler added a
goog.require() for it.*

How it works after my change:

* If I reference the extern in a library, the compiler adds a
goog.require() for it.
* If I reference the extern in an application, the compiler adds a
goog.require() for it.

I made the compiler's behavior more consistent because the it was already
adding goog.require() for externs in applications. If that's wrong, it was
already wrong before I made any changes.

--
Josh Tynjala
Bowler Hat LLC 



On Mon, Jul 8, 2019 at 11:26 PM Alex Harui  wrote:

> This doesn't seem right to me.  Externs should be representing JS files
> that don't have goog.provide in them.
>
> -Alex
>
> On 7/8/19, 1:40 PM, "joshtynj...@apache.org" 
> wrote:
>
> This is an automated email from the ASF dual-hosted git repository.
>
> joshtynjala pushed a commit to branch develop
> in repository
> 
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitbox.apache.org%2Frepos%2Fasf%2Froyale-compiler.gitdata=02%7C01%7Caharui%40adobe.com%7Cc589f5d7d32c425579c208d70488262a%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C1%7C636982855118961103sdata=C4JjqtBaAcFyhUCORG7jgDP3X0bthRGexYZVS%2B1A29Q%3Dreserved=0
>
>
> The following commit(s) were added to refs/heads/develop by this push:
>  new 942a423  PackageHeaderEmitter: output goog.require() for
> externs because release builds fail without it
> 942a423 is described below
>
> commit 942a4235758020239eca707a5e5c73e94a63df8c
> Author: Josh Tynjala 
> AuthorDate: Mon Jul 8 13:40:05 2019 -0700
>
> PackageHeaderEmitter: output goog.require() for externs because
> release builds fail without it
> ---
>  .../royale/compiler/internal/codegen/js/jx/PackageHeaderEmitter.java
>  | 3 ---
>  1 file changed, 3 deletions(-)
>
> diff --git
> 
a/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/jx/PackageHeaderEmitter.java
> 
b/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/jx/PackageHeaderEmitter.java
> index 7c8b9ac..57cfc76 100644
> ---
> 
a/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/jx/PackageHeaderEmitter.java
> +++
> 
b/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/jx/PackageHeaderEmitter.java
> @@ -380,9 +380,6 @@ public class PackageHeaderEmitter extends
> JSSubEmitter implements
>
>  if (imp.equals(cname))
>  continue;
> -
> -if (project.sourceExterns.contains(imp))
> -   continue;
>
>  if (NativeUtils.isNative(imp))
>  {
>
>
>
>

Re: [royale-compiler] branch develop updated: PackageHeaderEmitter: output goog.require() for externs because release builds fail without it

2019-07-09 Thread Josh Tynjala 
Without a goog.require(), the .js file for the extern does not seem to be
copied to the output folder, and Closure renames its APIs.

How the compiler behaved *before* my change:

* If I reference the extern in a library, the compiler did not add a
goog.require() for it.
* If I reference the extern in an application, *the compiler added a
goog.require() for it.*

How it works after my change:

* If I reference the extern in a library, the compiler adds a
goog.require() for it.
* If I reference the extern in an application, the compiler adds a
goog.require() for it.

I made the compiler's behavior more consistent because the it was already
adding goog.require() for externs in applications. If that's wrong, it was
already wrong before I made any changes.

--
Josh Tynjala
Bowler Hat LLC 


On Mon, Jul 8, 2019 at 11:26 PM Alex Harui  wrote:

> This doesn't seem right to me.  Externs should be representing JS files
> that don't have goog.provide in them.
>
> -Alex
>
> On 7/8/19, 1:40 PM, "joshtynj...@apache.org" 
> wrote:
>
> This is an automated email from the ASF dual-hosted git repository.
>
> joshtynjala pushed a commit to branch develop
> in repository
> https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitbox.apache.org%2Frepos%2Fasf%2Froyale-compiler.gitdata=02%7C01%7Caharui%40adobe.com%7C98ea3f4308e6419aa4c908d703e48029%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C1%7C636982152277066484sdata=K7kYU5xaJKz%2BXluJl0gpl0N21%2FlV%2FS6Z%2BfPEqaVWipg%3Dreserved=0
>
>
> The following commit(s) were added to refs/heads/develop by this push:
>  new 942a423  PackageHeaderEmitter: output goog.require() for
> externs because release builds fail without it
> 942a423 is described below
>
> commit 942a4235758020239eca707a5e5c73e94a63df8c
> Author: Josh Tynjala 
> AuthorDate: Mon Jul 8 13:40:05 2019 -0700
>
> PackageHeaderEmitter: output goog.require() for externs because
> release builds fail without it
> ---
>  .../royale/compiler/internal/codegen/js/jx/PackageHeaderEmitter.java
>  | 3 ---
>  1 file changed, 3 deletions(-)
>
> diff --git
> a/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/jx/PackageHeaderEmitter.java
> b/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/jx/PackageHeaderEmitter.java
> index 7c8b9ac..57cfc76 100644
> ---
> a/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/jx/PackageHeaderEmitter.java
> +++
> b/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/jx/PackageHeaderEmitter.java
> @@ -380,9 +380,6 @@ public class PackageHeaderEmitter extends
> JSSubEmitter implements
>
>  if (imp.equals(cname))
>  continue;
> -
> -if (project.sourceExterns.contains(imp))
> -   continue;
>
>  if (NativeUtils.isNative(imp))
>  {
>
>
>
>