Clojurians
#cljs-dev
<
2017-08-11
>

This page is not created by, affiliated with, or supported by Slack Technologies, Inc.

anmonteiro15:08:51

@dnolen what whitespace changes are you referring to in CLJS-2317?

anmonteiro15:08:14

oh I see it now

anmonteiro15:08:56

@dnolen also reworked the bootstrap script not to be dependent on that http://dl.google.com link in the patch that upgrades Closure Compiler: https://dev.clojure.org/jira/browse/CLJS-2316

anmonteiro15:08:16

now we can download it from Maven just like the other deps

wcohen16:08:49

Before I report this upstream, just want to make sure this looks like a new bug to you all who are more familiar with the compiler. closure-compiler master is failing on the following JS with --process_common_js_modules set:

var foo = function foo() {
  return 1;
};

module.exports = {
  foo: foo,
};
With error:
java.lang.RuntimeException: INTERNAL COMPILER ERROR.
Please report this problem.

EXPR_RESULT 1 [length: 3] [source_file: fails.js]
  Node(NAME foo): fails.js:1:4
var foo = function foo() {
  Parent(VAR): fails.js:1:0
var foo = function foo() {

	at com.google.common.base.Preconditions.checkState(Preconditions.java:444)
	at com.google.javascript.rhino.IR.assign(IR.java:424)
	at com.google.javascript.jscomp.ProcessCommonJSModules$RewriteModule.updateNameReference(ProcessCommonJSModules.java:1100)
	at com.google.javascript.jscomp.ProcessCommonJSModules$RewriteModule.maybeUpdateName(ProcessCommonJSModules.java:982)
	at com.google.javascript.jscomp.ProcessCommonJSModules$RewriteModule.visit(ProcessCommonJSModules.java:727)
...
	
The error only occurs when var foo and function foo() are both set to the same name.

anmonteiro16:08:52

cc @juhoteperi

dnolen19:08:13

@anmonteiro it appears CLJS-2316 needs a rebase?

juhoteperi19:08:30

@dnolen CLJS-2291 included the same fixes to index-node-modules-dir as my patch, so I'm quite sure it is not relevant

juhoteperi19:08:18

This one is still open: https://dev.clojure.org/jira/browse/CLJS-2294 (just noticed the description is missing some text, but I think the last comment explains the problems at least)

anmonteiro20:08:24

@dnolen rebasing CLJS-2316, will ping when done

anmonteiro20:08:28

CLJS-2288 can be closed

anmonteiro20:08:59

@dnolen replaced CLJS-2316's patch with an updated one

anmonteiro20:08:13

you’ll need to apply it with --keep-cr -p1

anmonteiro20:08:03

are you thinking about cutting a release today?

dnolen20:08:42

@anmonteiro yeah I forgot about --keep-cr what does -p1 do?

anmonteiro20:08:50

dunno was in my bash history

dnolen20:08:53

@anmonteiro I would like to but I’m not sure I need to fix these :modules issues

anmonteiro20:08:56

maybe only --keep-cr is needed

dnolen20:08:06

it applied with --keep-cr

anmonteiro20:08:34

I’ll still start trying out current master

anmonteiro20:08:01

I don’t have a project with :modules I can try anyway

dnolen21:08:24

yeah unlikely to get to a release today - the :modules foreign-lib thing is a bit trickier than expected

dnolen21:08:05

well, not so much tricky than the seemingly obvious thing wasn’t going to work and it took a while to figure that out

anmonteiro21:08:24

was just curious