bhauman / lein-figwheel

Figwheel builds your ClojureScript code and hot loads it into the browser as you are coding!
Eclipse Public License 1.0
2.88k stars 210 forks source link

Potentially Bad Configuration Validation #641

Closed zumbalogy closed 6 years ago

zumbalogy commented 6 years ago

It seems that Figwheel validates all the builds in a project.clj, even if a build is specified. This is not too bad in itself, though getting an error for a build other than the one specified is confusing. But this is problematic because Figwheel seems to think that builds that are valid are invalid, unless I am mistaken. Currently, a build with, say, ":optimizations :advanced" will cause Figwheel to error if ":source-map :none" is set. Removing the :source-map option fixes it, but it would maybe be better for Figwheel to not error here at all.

Steps to reproduce: lein new figwheel hello-world add ":source-map :none" to the "min" build run "lein figwheel" or "lein figwheel dev"

And the errors look like:

lein figwheel dev

Figwheel: Cutting some fruit, just a sec ...
Figwheel: Validating the configuration found in project.clj
------ Figwheel Configuration Error ------

Error at [:cljsbuild :builds 1 :compiler :source-map]
:source-map must be a string? when :optimizations is not :none

Figwheel: There are errors in your configuration file - project.clj

I attach this potentially gratuitous information in hopes that it may help someone find this if they are searching for it.

bhauman commented 6 years ago

https://clojurescript.org/reference/compiler-options#source-map

On Fri, Feb 2, 2018 at 10:02 PM, zumbalogy notifications@github.com wrote:

It seems that Figwheel validates all the builds in a project.clj, even if a build is specified. This is not too bad in itself, though getting an error for a build other than the one specified is confusing. But this is problematic because Figwheel seems to think that builds that are valid are invalid, unless I am mistaken. Currently, a build with, say, ":optimizations :advanced" will cause Figwheel to error if ":source-map :none" is set. Removing the :source-map option fixes it, but it would maybe be better for Figwheel to not error here at all.

Steps to reproduce: lein new figwheel hello-world add ":source-map :none" to the "min" build run "lein figwheel" or "lein figwheel dev"

And the errors look like:

lein figwheel dev

Figwheel: Cutting some fruit, just a sec ... Figwheel: Validating the configuration found in project.clj ------ Figwheel Configuration Error ------

Error at [:cljsbuild :builds 1 :compiler :source-map] :source-map must be a string? when :optimizations is not :none

Figwheel: There are errors in your configuration file - project.clj

I attach this potentially gratuitous information in hopes that it may help someone find this if they are searching for it.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/bhauman/lein-figwheel/issues/641, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAKQNZ_CVqqbJgFq7lPv_ADjK0HgaNaks5tQ8w8gaJpZM4R4BAY .

zumbalogy commented 6 years ago

I see, and while I still think that not allowing false is maybe a bit silly in this case (unless I am missing something), I apologize for hoisting my misunderstand upon you, and I see that its not Figwheel's fault. I would think that maybe it would be preferable for Figwheel to only verify the build specified, but I am certainly not in the position to make that decision.

Thank you for your time, and thank you so much for developing Figwheel!

A side note: the original situation holds for "false" not just ":none" which seems to be invalid in any case.