You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2021/04/28 05:01:08 UTC

[GitHub] [arrow] domoritz opened a new pull request #10183: ARROW-12393: [JS] [WIP] Drop closure compiler

domoritz opened a new pull request #10183:
URL: https://github.com/apache/arrow/pull/10183


   Before
   
   ```
   693k -I Arrow.es5.min.js.map
    55k -I Arrow.externs.js
   252k -I Arrow.js
   ```
   
   After
   
   ```
   1.2M -I Arrow.es5.min.js.map
   310k -I Arrow.js
   ```


-- 
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



[GitHub] [arrow] domoritz commented on pull request #10183: ARROW-12393: [JS] [WIP] Drop closure compiler

Posted by GitBox <gi...@apache.org>.
domoritz commented on pull request #10183:
URL: https://github.com/apache/arrow/pull/10183#issuecomment-828486502


   Ah yes, I switched away from that last month when we updated to the latest closure compiler version. 


-- 
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



[GitHub] [arrow] trxcllnt commented on pull request #10183: ARROW-12393: [JS] [WIP] Drop closure compiler

Posted by GitBox <gi...@apache.org>.
trxcllnt commented on pull request #10183:
URL: https://github.com/apache/arrow/pull/10183#issuecomment-829648616


   @domoritz it seems like not using Closure Compiler yields a bundle that's 58kb larger. Am I reading that correctly? What's the impact after compression? A larger bundle seems like a regression here.


-- 
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



[GitHub] [arrow] domoritz commented on pull request #10183: ARROW-12393: [JS] [WIP] Drop closure compiler

Posted by GitBox <gi...@apache.org>.
domoritz commented on pull request #10183:
URL: https://github.com/apache/arrow/pull/10183#issuecomment-829650469


   I haven't had the cycles to actually check. The bundles may not be correct yet so I wouldn't count on these numbers yet. 
   
   But if they are right, I wonder whether we should use closure for non es5 as well. 


-- 
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



[GitHub] [arrow] domoritz closed pull request #10183: ARROW-12393: [JS] [WIP] Drop closure compiler

Posted by GitBox <gi...@apache.org>.
domoritz closed pull request #10183:
URL: https://github.com/apache/arrow/pull/10183


   


-- 
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



[GitHub] [arrow] domoritz commented on pull request #10183: ARROW-12393: [JS] [WIP] Drop closure compiler

Posted by GitBox <gi...@apache.org>.
domoritz commented on pull request #10183:
URL: https://github.com/apache/arrow/pull/10183#issuecomment-828459710


   Thanks for offering to help. No, I don't think it's discontinued but it's a lot of work to keep it running for potentially small benefits so we are considering to remove it. We already use terser for all non es5 builds. 


-- 
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



[GitHub] [arrow] domoritz commented on a change in pull request #10183: ARROW-12393: [JS] [WIP] Drop closure compiler

Posted by GitBox <gi...@apache.org>.
domoritz commented on a change in pull request #10183:
URL: https://github.com/apache/arrow/pull/10183#discussion_r621817736



##########
File path: js/gulpfile.js
##########
@@ -46,13 +44,10 @@ for (const [target, format] of combinations([`all`], [`all`])) {
 // a minifier, so we special case that here.
 knownTargets.forEach((target) => {
     const umd = taskName(target, `umd`);
-    const cls = taskName(UMDSourceTargets[target], `cls`);
+    const esm = taskName(target, `esm`);
     gulp.task(`build:${umd}`, gulp.series(
-        `build:${cls}`,
-        `clean:${umd}`, `compile:${umd}`, `package:${umd}`,
-        function remove_closure_tmp_files() {
-            return del(targetDir(target, `cls`))
-        }
+        `build:${esm}`,

Review comment:
       @trxcllnt I don't know much about gulp but this seems to run the same job twice when I run `yarn build -t es5`. How can I avoid 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



[GitHub] [arrow] domoritz commented on pull request #10183: ARROW-12393: [JS] [WIP] Drop closure compiler

Posted by GitBox <gi...@apache.org>.
domoritz commented on pull request #10183:
URL: https://github.com/apache/arrow/pull/10183#issuecomment-835606300


   Will redo and compare with updated terser as well. 


-- 
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



[GitHub] [arrow] alippai commented on pull request #10183: ARROW-12393: [JS] [WIP] Drop closure compiler

Posted by GitBox <gi...@apache.org>.
alippai commented on pull request #10183:
URL: https://github.com/apache/arrow/pull/10183#issuecomment-828464302


   https://github.com/google/closure-compiler-npm/blob/master/packages/google-closure-compiler-js/readme.md, you are right, it looks like only the JS version is being discontinued


-- 
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



[GitHub] [arrow] alippai commented on pull request #10183: ARROW-12393: [JS] [WIP] Drop closure compiler

Posted by GitBox <gi...@apache.org>.
alippai commented on pull request #10183:
URL: https://github.com/apache/arrow/pull/10183#issuecomment-828224521


   @domoritz closure compiler is discontinued, right? I can help looking into alternatives if you need any help with this issue.


-- 
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



[GitHub] [arrow] domoritz commented on pull request #10183: ARROW-12393: [JS] [WIP] Drop closure compiler

Posted by GitBox <gi...@apache.org>.
domoritz commented on pull request #10183:
URL: https://github.com/apache/arrow/pull/10183#issuecomment-828148657


   Maybe it's worth trying to run closure over the es2015 sources as well (instead of removing closure) if the results are that much better 😆. 


-- 
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



[GitHub] [arrow] github-actions[bot] commented on pull request #10183: ARROW-12393: [JS] [WIP] Drop closure compiler

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #10183:
URL: https://github.com/apache/arrow/pull/10183#issuecomment-828144743


   https://issues.apache.org/jira/browse/ARROW-12393


-- 
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