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

[Enhancement]: Warning on re-registering subs / events (ie name collision detection) improvements #769

Closed
sirmspencer opened this issue Nov 30, 2022 · 4 comments

Comments

@sirmspencer
Copy link

sirmspencer commented Nov 30, 2022

  1. It would be great to have the message show for a newly created sub and a hot reload. We work in shadow-cljs and rarely do a full reload during dev. I am not sure if this is possible, since a hot reload could just end up generating warnings for every sub in the file.

2. Setting to make the message a full error. We don't have any use cases for re adding subs, so an error is more apt.

3. A flag to fail during compilation would also be a great safety net. Basically, we would want it to fail in the ci deployment when we run tests, or build the ubar jar. Maybe a function that can check for duplicates, or any other warnings, that I can add to the test suite.

@sirmspencer sirmspencer changed the title [Enhancement]: Warning on re-registering subs / events (ie name collision detection) [Enhancement]: Warning on re-registering subs / events (ie name collision detection) improvements Nov 30, 2022
@sirmspencer
Copy link
Author

sirmspencer commented Nov 30, 2022

It looks like I can do #2 with set-loggers! For anyone using shadow the key for both is to call set-loggers from a preload in dev.

(ns sphere.utils.dev-preloads
  (:require [re-frame.core :as rf]))

;; re-frame logging
(defn escalate-re-frame-warn [& r]
  (if (= "re-frame: overwriting" (-> r js->clj first))
    (apply js/console.error r)
    (apply js/console.warn r)))

(re-frame.core/set-loggers! {:warn escalate-re-frame-warn})

@sirmspencer
Copy link
Author

sirmspencer commented Dec 1, 2022

I was also able to #3 with set-loggers!. For anyone using shadow the key for both is to call set-loggers! from a preload.

(ns sphere.utils.ci-preloads
  (:require [re-frame.core :as rf]))

(defn escalate-re-frame-warn [& r]
  (assert (not (= "re-frame: overwriting" (-> r js->clj first)))
          (apply str r)))

(defn escalate-re-frame-error [& r]
  (assert false
          (apply str r)))

(re-frame.core/set-loggers! {:warn escalate-re-frame-warn
                             :error escalate-re-frame-error})

@mike-thompson-day8
Copy link
Contributor

mike-thompson-day8 commented Dec 1, 2022

Regarding point 1, re-frame will warn about duplicates on the initial loading of code (up to the window "load" event) but after that, no warnings are issued (during hot code reloading). See #204

@sirmspencer
Copy link
Author

Regarding point 1, re-frame will warn about duplicates on the initial loading of code (up to the window "load" event) but after that, no warnings are issued (during hot code reloading). See #204

I had a guess that might be the case

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants