search

duct-framework / server.figwheel

Integrant methods for running Figwheel
4 stars 8 forks source link

NullPointerException when processing externs files #7

Open iarenaza opened 5 years ago

iarenaza commented 5 years ago

lein-figwheel expects files with .js extension inside its source directories to be foreign libraries. And foreign libraries must declare a namespace. In fact, lein-figwheel assumes it, and if it doesn't find it and can't map the file back to a source .cljs file, it bombs out with a NullPointerException when trying to check for its existance.

This might happen when you put your externs file(s) inside the source directories (this is our case, e.g. https://github.com/magnetcoop/hydrogen-ce/blob/master/src/hyd/client/externs.js)

While lein-figwheel doesn't by default try to process such files on its own, when using Duct server.figwheel it tells lein-figwheel to proccess all files inside the configured source directories (see https://github.com/duct-framework/server.figwheel/blob/master/src/duct/server/figwheel.clj#L54-L55).

Clearly lein-figwheel should be more robust and handle that situation in a more graceful way[1]. But on the other hand Duct server.figwheel shouldn't be telling lein-figwheel to process absolutely all files in source directories (but probably just those having .cljs/cljc extension and those declared as foreign libraries).

[1] We have opened a pull request in lein-figwheel regarding this behaviour, see https://github.com/bhauman/lein-figwheel/pull/739

weavejester commented 5 years ago

Thanks for the issue report. Changing the the default behavior to process .cljs and .cljc files only seems a sensible idea, as long as there's an option to change it.

iarenaza commented 5 years ago

@weavejester Which kind of option did you have in mind? A boolean switch for "proccess all files" vs. "process only cljs/cljc" files? Or something completely flexible where the user could specify specific subsets of files? (e.g., providing some matching criteria)

weavejester commented 5 years ago

A regular expression for matching filenames seems reasonable to me. Maybe Figwheel already has that? Or at least has a filtering function?

iarenaza commented 3 years ago

@weavejester What do you think about this patch? d34312b4d51120014ac66183d5e56e2ddfdb495d

weavejester commented 3 years ago

@weavejester What do you think about this patch? d34312b

There are a few rough edges that would need to be sanded down, but otherwise that looks good. Submit it as a PR.