Navigation Menu

Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

If the namespace has a reserved JS keyword in its name, cljs.compiler will make figwheel look for the wrong path #120

Closed
ravicious opened this issue Apr 3, 2015 · 4 comments

Comments

@ravicious
Copy link

  • figwheel 0.2.5-SNAPSHOT
  • clojure 1.6.0
  • clojurescript 0.0-3058
  • Google Chrome 41.0.2272.104 (64-bit) / Mozilla Firefox 36.0.4
  • OS X 10.10.2

A repo with an example that illustrates the problem.

Steps to reproduce the problem:

  1. Clone the repo.
  2. Run figwheel with lein figwheel.
  3. Open the app page.
  4. Open the developer console.
  5. Open src/figwheel_namespace_bug/interface/handlers.cljs and trigger a file reload (just saving the file should do the job).
  6. figwheel will output something like that in the console:
Figwheel: notified of file changes
❌ GET http://localhost:3449/js/compiled/out/figwheel_namespace_bug/interface$/handlers.js?zx=8mdlcix66wpj
❌ Figwheel: Error loading file http://localhost:3449/js/compiled/out/figwheel_namespace_bug/interface$/handlers.js

As you can see, instead of trying to look in the interface folder, it tries to fetch the interface$ folder.

Current solution: just rename the namespace to ui. ;)

ravicious added a commit to ravicious/ultimate-ttt-cljs that referenced this issue Apr 3, 2015
This change circumvents the bug in figwheel due to which it won't reload files that are in a namespace that has `interface` in its name.

bhauman/lein-figwheel#120
@ravicious
Copy link
Author

After digging into the source code and turning on the debug mode, I think I found the culprit: figwheel-sidecar.core/make-sendable-file uses cljs.compiler/munge, which adds a $ to reserved JS words.

MDN docs state that interface is indeed a reserved JS keyword.

munge takes a set of reserved keywords as the second argument and if it's not provided, it uses this set from cljs.analyzer.

One possible solution that popped into my head: let's pass an empty set to munge (though I don't know what consequences it has for the rest of figwheel).

@ravicious ravicious changed the title If the namespace has interface in its name, figwheel looks for the folder interface$ instead of interface If the namespace has a reserved JS keyword in its name, cljs.compiler will make figwheel look for the wrong path Apr 4, 2015
@ravicious
Copy link
Author

Looks like the commit 212a881 introduces cljs.compiler/munge. Guessing from the code and my understanding of what munge does, using an empty set shouldn't cause any problems.

@ravicious
Copy link
Author

I investigated this issue further and passing an empty set to munge doesn't fix the problem entirely. It makes the file visible for figwheel, but something's still wrong:

Figwheel: notified of file changes
Figwheel: Not trying to load file http://localhost:3449/js/compiled/out/figwheel_namespace_bug/interface/handlers.js
FigWheel: Attempting to load http://localhost:3449/js/compiled/out/figwheel_namespace_bug/core.js
FigWheel: Successfullly loaded http://localhost:3449/js/compiled/out/figwheel_namespace_bug/core.js
Figwheel: loaded these files
("figwheel_namespace_bug/core.js")
Figwheel: NOT loading these files 
not required: ("/figwheel_namespace_bug/interface/handlers.js")

By following these logs I managed to find out that figwheel-namespace-bug.interface.handlers must be ruled out by the figwheel-client.file-reloading/reload-file? function.

Turns out that figwheel-namespace-bug.interface.handlers was not in the *loaded-libs* set. So I found the only place where *loaded-libs* is touched in any way: patch-goog-base.

I put a debugger in the overwritten js/goog require function and found out that the name that gets sent to it is still munged. Call stack showed that there was only one function call before that and it pointed to the ns statement in my core.cljs file, so I guess that had to be cljs.compiler munging stuff by itself and if you look at its sources it's true – they use munge in a lot of places.

So it got me thinking – I can't be the first to have this problem. Quick googling reveled that David Nolen reported this issue back in January 2014 and added the warning in clojure/clojurescript@b0b135b0 22 days ago.

I think unless something changes in ClojureScript, people should be discouraged from using reserved JS keywords in namespaces, so this kinda solves my issue, but I'm leaving it open in case @bhauman wants to add anything.

@bhauman
Copy link
Owner

bhauman commented Apr 5, 2015

Thanks for looking into this. Yeah using reserved keywords in namespaces is a bad idea right now, and maybe a bad idea in general. :)

@bhauman bhauman closed this as completed Apr 5, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants