Logging library

[Wendystraite]

Hello, I am working on a logging library for Zedux : https://github.com/Wendystraite/zedux-logger

It is supposed to work with the latest beta version of Zedux using the new plugin system.

If you have some time to test it I appreciate any feedback.

Notable features :

• Diffing old/new state of objects/arrays
• Graph grouped by namespaces
• Snapshot of the entire ecosystem
• Deobfuscting the scary single letters
• Emojis and colors
• Showing promise's state and dependent atoms that are waiting for it

To test it : import { addZeduxLogger } and call it with as first parameter the ecosystem and as second parameter the logger options.

[bowheart]

Wooww this is pretty extensive, @Wendystraite nice work 👏. Amazing to see the new plugin system in action. The simple demos I've put together aren't anywhere near this comprehensive.

Sure, I have some notes.

First, a couple things going out in the next release, I'm sure you already know given your comment on #190

• flags are renamed to tags (feat(atoms)!: rename atom flags to tags #188)
• there's no longer a concept of ecosystem destruction, so the isDestroy property on reset* events is gone, replaced with hydrations, listeners, and overrides properties.

I'm currently finalizing all API changes for the first release candidate, so this will all be considered stable very soon. But first, I'm planning on changing a few more things:

• (the biggest one) renaming GraphNode to ZeduxNode. Playtesting Zedux the last few weeks, I've found GraphNode too generic.
• "dependencies" -> "sources" and "dependents" -> "observers" in the objects returned from ecosystem.viewGraph. This is consistent with Zedux v2's nomenclature.
• _promiseStatus -> promiseStatus (same thing for _promiseError). These are properties users should be able to read; they shouldn't be underscore-prefixed.

I see a few things Zedux can improve.

• Some event types like promiseChange are guaranteed to always have a source property. We should change that so you don't need to check if promiseChange?.source !== undefined
• I can export an EcosystemEventType type so you don't have to do EcosystemEvents[keyof EcosystemEvents]['type']

A few things the logger can improve:

• generateSnapshot and generateGraph are pretty expensive to be running on every ecosystem event. I know at least three of our apps have too many events for that. The ecosystem events are designed such that those can be incrementally modified as events come in.
• A feature request! The primary purpose of runStart and runEnd is to track run time. If handleEventRunEnd output that, especially colored red if it's relatively slow, I'd honestly start using this right now.

The deobfuscation stuff is a candidate for the official @zedux/utils package (#143). I think it makes sense for us to maintain that, since all single-letter properties are internal implementation details. Though they are stable for exactly this reason - so addon packages can use them. Just a thought, I'm not worrying about that right now.

The single letters are scary, and I wish there was a way to make autocomplete and debuggers show them last. Switching to them has trimmed more than 5kb off the minified build, so they're pretty hard to argue against. I'm glad I've been diligent about commenting those so you knew what they were 😄 .

Some properties are probably unnecessarily obfuscated. I'm planning on revisiting them over time as I actually use Zedux v2 - adding user-friendly getters for properties that end-users do need to know about. That can be done in minor versions. Let me know if you've encountered any you really wish weren't obfuscated.

Anyway, brilliant work. If you publish this, I'll add a mention/link to it in the docs 🙌

[Wendystraite]

Thank you for the reply :)

Yes it's already published on npm here but as you said not ready to be used in big projets.

The renamings are alright, I was asking myself white coding if I was doing something bad to use underscore/private properties to check the promises states 😄
Related to that I think I encountered a timing issue while working with lot of promises where the last log was saying that an atom was waiting for multiple promises but no new log to notify that It was finished. Might be a bug from my side, I will keep you posted If I repro this.

generateSnapshot and generateGraph are my 1st priority yes, I will add a few tests with these two.
As for your use case of runStart and end that's a good use case, thank you.

I have also planned to :

• Revisit the options for filtering
  • Either be able to add multiple logger with different options or be able to give an array of options to a single logger
    • With option 1 I would put the loggers' state inside ecosystem._storage, I think that's a good use ?
  • Combined with that a way to include/exclude atom names / namespaces / tags / types / ..
  • And some options to customize everything or have a set of templates/options already made.
  • The idea behind this is to be able to log for example everything with less vibrant colors except a namespace of atoms that you want to debug.
  • Another idea is to disable a lot of logging for intensive atoms and be able to have the "Group similar" devtool feature kicking in for them.
• Have a way to disable colors and maybe disable the group (display an object at the end instead). This could be useful for downloading the logs and having only one line per log.
• Maybe revisit the "reasons" given. Sometimes the logger doesn't show any "reasons" (empty array) for something but should show something. I might experiment a bit to find a good balance.
• An API to build or insert custom logs. Might work on that after every other features.

I haven't seen something that shouldn't be obfuscated but I have seen this :

• the "Listener" node is not exported from the package, that's an advanced feature so this might be debatable.

[bowheart]

Incremental generateSnapshot and generateGraph are also possibly candidates for the @zedux/utils package. I'm sure lots of plugins will want those abilities.

ecosystem._storage is good for that. Just namespace your storage key(s).

If you want users to be able to configure logger state on the fly, you can export a full-on atom. Just make sure you don't update it every time an ecosystem event comes in or you'll have an infinite update loop.

A way to provide thresholds for what's considered slow or update-happy would be cool.

I just found the parseAtomName function. We can definitely better standardize id structure 😅 . double-@@selector vs single-@signal bugs me. The no- prefix is out of left field. Hmm yeah I think I will try to improve those and I'll let you know. That's easy to do and it's a great time to slip those changes in.

Exporting Listener is a good shout. Since GraphNode itself references that on the stable, public Listener property, it should be exported. Instantiating it standalone is a very advanced feature that I'm not even sure I want to document. But regardless, Listener should be exported for types alone.

[bowheart]

@Wendystraite FYI I've opened #201 to standardize id generation.

[bowheart]

@Wendystraite I'm thinking of making a change to ecosystem events that I don't think will functionally impact the logger, but want to run it by you just in case:

Currently ecosystem events are scheduled. This ensures plugins don't receive events during React renders. But it has (minor) problems compared to the alternative approach of passing them to event listeners immediately with no scheduling step:

• Overhead. Scheduling extra events slows down all other graph operations. It shouldn't be significant, but it's probably the difference between me recommending removing ecosystem event listeners in prod and saying they're fine.
• Callstack obfuscation. If events fired immediately, you'd be able to insert a breakpoint in a cycle event listener, for example, and trace it back to see exactly why an atom was destroyed. That isn't possible with the current approach.
• Some events need to run immediately anyway. runStart and runEnd in particular need to be changed to not be scheduled so the run time tracking I previously mentioned actually works. This means plugin authors need to be aware of their callbacks potentially running during React render anyway. That ruins the main reason I wanted ecosystem events to be scheduled in the first place - so they can update state immediately without worrying about React warnings from updating a component while another component is rendering.

So rather than change some events to not be scheduled, I'm thinking of changing them all to not be scheduled. I see you have a note about performance issues in the logger readme. I didn't dig into it, but this change could help.

Plugin authors will just need to be aware that they can receive events during React renders. Update state in a microtask to be safe.

Do you know of any way this change could negatively impact the logger?

[Wendystraite]

Hello @bowheart, I was planning to write here after merging my v2 branch for the logger yesterday :)

Do you know of any way this change could negatively impact the logger?

I don't think this would so no worries for that.
The logger stores his state in zedux ecosystem's _storage.. Some properties are updated during events but are not used outside of it*. The only possible negative impact would be on the logger's exported atom zeduxLoggerAtom. This atom export this data outside in a signal but I haven't tested yet yo use it inside React.

I see you have a note about performance issues in the logger readme. I didn't dig into it, but this change could help.

I don't think this would for the current performance problems of the logger. The main problem is about the graph generation - incremental or not - as you can see here.

---

About zedux's v2, the migration went well, the new node ids are a welcome change, this simplified a lot of logic. Loving what's being done.

I mainly encountered 3 problems:

• When generating the incremental graph/snapshot, during tests, I was checking his consistency based on the ecosystem viewGraph method and I saw that ecosystem.n did not contain every graph nodes. This looks intentional because of your comment on the method but when digging zedux's code, the ecosystem.findAll method is using ecosystem.n. This result in some nodes not being returns by this method (for example @memo nodes). This mainly affects the logger's graph/snapshot generation so for now the logger ignores every @memo nodes (code here).

• Also about the graph. This might be fixed without events scheduling. Basically when receiving an event, in older code, I assumed that the ecosystem graph was stable to use but discovered that it (ecosystem.n) was actually not always immediately updated based on the event. For example, when receiving a CycleChange event with the status active, the new node is not returned by viewGraph. It is correctly returned during the next event. My trick was to delay it after all events. This should be documented for libraries authors.

• Finally about React Strict mode used with Zedux. When enabled, this breaks all interactions in the logger's playground so I had to disable it here. I haven't investigated much before updating Zedux but the problem seems to persist. Do you want me to create a new issue with a codesandbox about this ?

[bowheart]

I don't think I realized that @memo nodes aren't added to ecosystem.n. That would be the only node type that isn't, which is weird. I'll change that. Good find.

I'm not sure if it's possible to make sure that the ecosystem's internal state always reflects what plugins receive right when ecosystem events are sent. But I'll do a pass through the code and see if I can improve some of them. I'll make sure any exceptions are documented.

No need to create a sandbox. I found the issue, fixed it in #232, and published the fix in version 2.0.0-rc.5. React StrictMode will unfortunately always be less battle tested 'cause we don't use it. I just missed something simple, but it at least has a regression test now. I also manually tested the fix briefly in your logger playground.

In this version, you may need to be aware of #231 too, which added an R property to ZeduxNode that you may need to call to force-resolve a node's weight before logging it.