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

Add dynamic var for warning on handler overwrite #413

Closed
wants to merge 1 commit into from

Conversation

danielcompton
Copy link
Contributor

When figwheel reloads code, it causes handlers to be re-registered which leads to a warning for each handler. This leads to noisy logs. Figwheel has a :before-jsload key which can be called before it reloads application code.

This patch adds a *warn-on-overwrite* dynamic var, and shows how to use it in the example project.

One side effect of using this is that you will never get a warning for duplicate handlers when Figwheel is reloading. It may be possible to track how many handlers were reloaded in a particular Figwheel reload, but this would get very complex. In any case, you will still get warnings every time you refresh the browser, which should be good enough for the relatively rare case of creating duplicate handlers.

This still needs

  • tests
  • a decision on where to put the dynamic var (in re-frame.core or elsewhere?, if in core, what about circular dependencies?)
  • Recommendations on the usage of dynamic vars
  • Is :before-jsload supported/able to be used?
  • Update changelog
  • Docs on usage

Fixes #204

When figwheel reloads code, it causes handlers to be re-registered
which leads to a warning for each handler. This leads to noisy logs.
Figwheel has a :before-jsload key which can be called before it reloads
application code.

This patch adds a *warn-on-overwrite* dynamic var, and shows how to use
it in the example project.

One side effect of using this is that you will never get a warning
for duplicate handlers when Figwheel is reloading. It may be possible
to track how many handlers were reloaded in a particular Figwheel
reload, but this would get very complex. In any case, you will still
get warnings every time you refresh the browser, which should be good
enough for the relatively rare case of creating duplicate handlers.

Fixes #204
@danielcompton
Copy link
Contributor Author

I've gotten a bit blocked by bhauman/lein-figwheel#485. I think in the meantime we can release the method we've got above, and if a better option is introduced for something like with-jsload in the future, add support for it later.

@mike-thompson-day8
Copy link
Contributor

@danielcompton a reminder: when discussing this recently, we realised that there are two distinct phases:

  1. The period prior to first (figwheel ?) code reload
  2. there after

We want warnings during 1, but never 2.

So if we can catch the first (figwheel?) reload, then we can flip a switch so these warnings are not then reported.

@superstructor
Copy link
Contributor

Superseded. See #204. Thanks for the PR all the same 😄 Closing.

@superstructor superstructor deleted the warn-on-overwrite branch May 6, 2020 01:51
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

Successfully merging this pull request may close these issues.

Don't warn about reloading handlers when initiated by figwheel
3 participants