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

Improves readability of code examples #224

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ const setCounter = (value) => {
render();
};

const isEven = () => (counter & 1) == 0;
const isEven = () => (counter & 1) === 0;
const parity = () => isEven() ? "even" : "odd";
const render = () => element.innerText = parity();

Expand Down Expand Up @@ -75,7 +75,7 @@ To understand Signals, let's take a look at the above example, re-imagined with

```js
const counter = new Signal.State(0);
const isEven = new Signal.Computed(() => (counter.get() & 1) == 0);
const isEven = new Signal.Computed(() => (counter.get() & 1) === 0);
const parity = new Signal.Computed(() => isEven.get() ? "even" : "odd");

// A library or framework defines effects based on other Signal primitives
Expand Down Expand Up @@ -355,12 +355,12 @@ The `Watcher` interface defined above gives the basis for implementing typical J
// NOTE: This scheduling logic is too basic to be useful. Do not copy/paste.
let pending = false;

let w = new Signal.subtle.Watcher(() => {
const w = new Signal.subtle.Watcher(() => {
if (!pending) {
pending = true;
queueMicrotask(() => {
pending = false;
for (let s of w.getPending()) s.get();
for (let s of w.getPending()) { s.get(); }
tomasbonco marked this conversation as resolved.
Show resolved Hide resolved
w.watch();
});
}
Expand All @@ -370,7 +370,7 @@ let w = new Signal.subtle.Watcher(() => {
// itself on the microtask queue whenever one of its dependencies might change
export function effect(cb) {
let destructor;
let c = new Signal.Computed(() => { destructor?.(); destructor = cb(); });
const c = new Signal.Computed(() => { destructor?.(); destructor = cb(); });

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same should we go multiline here ?

Copy link
Author

@tomasbonco tomasbonco May 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I know this is sort of personal taste, but in my opinion this is acceptable. Lambda functions are often one-liners and the content in this case is not too complex. I assume a reader is used to reading the rest of the line. Understanding the purpose of the code is going to be the issue, but not its style.

w.watch(c);
c.get();
return () => { destructor?.(); w.unwatch(c) };
Expand Down