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

Checklist: exercises that could be implemented #238

Open
SleeplessByte opened this issue Apr 4, 2019 · 10 comments
Open

Checklist: exercises that could be implemented #238

SleeplessByte opened this issue Apr 4, 2019 · 10 comments
Labels
good first issue First-time contributors preferred help wanted

Comments

@SleeplessByte
Copy link
Member

SleeplessByte commented Apr 4, 2019

https://tracks.exercism.io/typescript/main/unimplemented

@SleeplessByte SleeplessByte added good first issue First-time contributors preferred help wanted labels Apr 12, 2019
@msomji
Copy link
Contributor

msomji commented Oct 23, 2019

created PR #297 for word-search

msomji added a commit to msomji/typescript that referenced this issue Oct 23, 2019
Added a new Exercise per issue exercism#238
msomji added a commit to msomji/typescript that referenced this issue Oct 23, 2019
Added a new Exercise per issue exercism#238
Roshanjossey pushed a commit to Roshanjossey/typescript that referenced this issue Oct 26, 2019
Roshanjossey pushed a commit to Roshanjossey/typescript that referenced this issue Oct 26, 2019
@msomji
Copy link
Contributor

msomji commented Nov 29, 2019

created PR #305 for Resistor-color-duo

SleeplessByte pushed a commit to Roshanjossey/typescript that referenced this issue Dec 14, 2020
SleeplessByte pushed a commit to Roshanjossey/typescript that referenced this issue Dec 14, 2020
SleeplessByte added a commit that referenced this issue Dec 14, 2020
* Add exercise resistor color

Address issue #238

* Add entry for resitor color in config.json

* Sync this exercise

Co-authored-by: rjossy <[email protected]>
Co-authored-by: Derk-Jan Karrenbeld <[email protected]>
@peerreynders
Copy link
Contributor

peerreynders commented Dec 19, 2020

In reference to the react exercise, I wonder whether the following tests would be taking too many liberties with the canonical problem specification:

react.test.ts

import { createInput, createComputed, createCallback } from './react'

describe('React module', () => {
  // c51ee736-d001-4f30-88d1-0c8e8b43cd07
  it('input cells have a value', () => {
    const initialValue = 10
    const [input, _setInput] = createInput(initialValue)
    expect(input()).toEqual(initialValue)
  })

  // dedf0fe0-da0c-4d5d-a582-ffaf5f4d0851
  it("an input cell's value can be set", () => {
    const newValue = 20
    const [input, setInput] = createInput(4)
    setInput(newValue)
    expect(input()).toEqual(newValue)
  })

  // 5854b975-f545-4f93-8968-cc324cde746e
  it('compute cells calculate initial value', () => {
    const [input] = createInput(1)
    const output = createComputed(() => input() + 1)
    expect(output()).toEqual(2)
  })

  // 25795a3d-b86c-4e91-abe7-1c340e71560c
  it('compute cell takes inputs in correct order', () => {
    const [[one], [two]] = [createInput(1), createInput(2)]
    const output = createComputed(() => one() + two() * 10)
    expect(output()).toEqual(21)
  })

  // c62689bf-7be5-41bb-b9f8-65178ef3e8ba
  it('compute cells update value when inputs are changed', () => {
    const [input, setInput] = createInput(1)
    const output = createComputed(() => input() + 1)
    setInput(3)
    expect(output()).toEqual(4)
  })

  // 5ff36b09-0a88-48d4-b7f8-69dcf3feea40
  it('compute cells can depend on other compute cells', () => {
    const [input, setInput] = createInput(1)
    const timesTwo = createComputed(() => input() * 2)
    const timesThirty = createComputed(() => input() * 30)
    const sum = createComputed(() => timesTwo() + timesThirty())
    expect(sum()).toEqual(32)
    setInput(3)
    expect(sum()).toEqual(96)
  })

  // abe33eaf-68ad-42a5-b728-05519ca88d2d
  it('compute cells fire callbacks', () => {
    const [input, setInput] = createInput(1)
    const output = createComputed(() => input() + 1)
    let value = 0
    createCallback(() => (value = output()))
    setInput(3)
    expect(value).toEqual(4)
  })

  // 9e5cb3a4-78e5-4290-80f8-a78612c52db2
  it('callbacks fire only when output values change', () => {
    const [input, setInput] = createInput(1)
    const output = createComputed(
      () => (input() < 3 ? 111 : 222),
      undefined,
      true // i.e. equality check - don't propagate if value doesn't change
    )
    let value: number | undefined
    createCallback(() => (value = output()))
    value = undefined // discard initial value from registration
    setInput(2)
    expect(value).toBeUndefined()
    setInput(4)
    expect(value).toEqual(222)
  })

  // ada17cb6-7332-448a-b934-e3d7495c13d
  it('callbacks do not report already reported values', () => {
    const [input, setInput] = createInput(1)
    const output = createComputed(() => input() + 1)

    let value: number | undefined
    createCallback(() => (value = output()))

    setInput(2)
    expect(value).toEqual(3)

    setInput(3)
    expect(value).toEqual(4)
  })

  // ac271900-ea5c-461c-9add-eeebcb8c03e5
  it('callbacks can fire from multiple cells', () => {
    const [input, setInput] = createInput(1)
    const plus_one = createComputed(() => input() + 1)
    const minus_one = createComputed(() => input() - 1)

    let value1: number = 0
    createCallback(() => (value1 = plus_one()))
    let value2: number = 0
    createCallback(() => (value2 = minus_one()))

    setInput(10)
    expect(value1).toEqual(11)
    expect(value2).toEqual(9)
  })

  // From JavaScript track
  it('static callbacks fire even if their own value has not changed', () => {
    const [input, setInput] = createInput(1)
    const output = createComputed(
      () => (input() < 3 ? 111 : 222),
      undefined,
      true // i.e. equality check - don't propagate if value doesn't change
    )
    const values: string[] = []
    createCallback(() => {
      const _dontCare = output()
      values.push('cell changed')
    })
    values.pop() // discard initial value from registration
    setInput(2)
    expect(values).toEqual([])
    setInput(4)
    setInput(2)
    setInput(4)
    expect(values).toEqual(['cell changed', 'cell changed', 'cell changed'])
  })

  // 95a82dcc-8280-4de3-a4cd-4f19a84e3d6f
  it('callbacks can be added and removed', () => {
    const [input, setInput] = createInput(11)
    const output = createComputed(() => input() + 1)

    const values1: number[] = []
    const unsubscribe1 = createCallback(() => values1.push(output()))
    values1.pop() // discard initial value from registration
    const values2: number[] = []
    createCallback(() => values2.push(output()))
    values2.pop() // discard initial value ...

    setInput(31)

    unsubscribe1()

    const values3: number[] = []
    createCallback(() => values3.push(output()))
    values3.pop() // discard initial value ...

    setInput(41)

    expect(values1).toEqual([32])
    expect(values2).toEqual([32, 42])
    expect(values3).toEqual([42])
  })

  // f2a7b445-f783-4e0e-8393-469ab4915f2a
  it("removing a callback multiple times doesn't interfere with other callbacks", () => {
    const [input, setInput] = createInput(1)
    const output = createComputed(() => input() + 1)

    const values1: number[] = []
    const unsubscribe1 = createCallback(() => values1.push(output()))
    values1.pop() // discard initial value from registration
    const values2: number[] = []
    createCallback(() => values2.push(output()))
    values2.pop() // discard initial value ...

    unsubscribe1()
    unsubscribe1()
    unsubscribe1()

    setInput(2)

    expect(values1).toEqual([])
    expect(values2).toEqual([3])
  })

  // daf6feca-09e0-4ce5-801d-770ddfe1c268
  it('callbacks should only be called once, even if multiple dependencies change', () => {
    const [input, setInput] = createInput(1)
    const plusOne = createComputed(() => input() + 1)
    const minusOne1 = createComputed(() => input() - 1)
    const minusOne2 = createComputed(() => minusOne1() - 1)
    const output = createComputed(() => plusOne() * minusOne2())

    const values: number[] = []
    createCallback(() => values.push(output()))
    values.pop() // discard initial value from registration

    setInput(4)

    expect(values).toEqual([10])
  })

  // 9a5b159f-b7aa-4729-807e-f1c38a46d377
  it("callbacks should not be called if dependencies change but output value doesn't change", () => {
    const [input, setInput] = createInput(1)
    const plusOne = createComputed(() => input() + 1)
    const minusOne = createComputed(() => input() - 1)
    const alwaysTwo = createComputed(
      () => plusOne() - minusOne(),
      undefined,
      true // i.e. equality check - don't propagate if value doesn't change
    )

    const values: number[] = []
    createCallback(() => values.push(alwaysTwo()))
    values.pop() // discard initial value from registration

    setInput(2)
    setInput(3)
    setInput(4)
    setInput(5)

    expect(values).toEqual([])
  })
})

The problem specification is rather object centric and the JavaScript track already has that style of tests - so I wondered whether having a more closure-oriented approach with TypeScript might be a change of pace.

The inspiration came from
https://indepth.dev/posts/1269/finding-fine-grained-reactive-programming#comparison-by-example
(pure Fine-Grained binding).

@SleeplessByte
Copy link
Member Author

It's a lot like react hooks; I like how it looks. I think it's fine to do the implementation like that @peerreynders .

Are you up for implementing that exercise?

@peerreynders
Copy link
Contributor

I was giving it a go.

I've got the tests and the example implementation and was in the process of double checking the tests against the problem specification.

That is when I noticed that the "comments" (specifications) were a lot more detailed than the exercise's readme - for example:

Each object in the cells array has a name and type (input or output).
"input cells have an initial_value, and compute cells have inputs and compute_function

My createComputed doesn't take inputs and createCallback is independent of a computed (derived) - so really one could argue that my implementation and tests aren't a good fit for the exercise.

The main differences are:

  • createInput returns two functions: an accessor and a mutator.
  • createComputeds only takes a function. When that function runs and that function invokes an input's accessor the Computed is subscribed to the input.
  • createCallback also only takes a function which can use an input's or computed's accessor. Nothing can depend on a callback so they are processed last (internally they are marked as 'impure' while computeds are 'pure'). However when that callback is registered the function runs to obtain its first value - and more importantly - to get subscriptions to its dependencies. It's because of this it is necessary that the actual value or array needs to be cleared of the initial value after the callback is registered.
  • In most reactive systems "events" are more important than "values" - otherwise the principle of least surprise regarding "events" is violated in preference to "values" (example in Svelte). So by default new values are reported even when they are equal to the previous value. So for tests that require a different behavior (no event when value does not change) the computed needs to be configured that way.

createInput, createComputed and createCallback are actually based on createSignal, createMemo and createEffect from Solid which aren't subject to hook rules.

@SleeplessByte
Copy link
Member Author

SleeplessByte commented Dec 19, 2020

This may help a bit. The original exercise canonical data was based on these four implementations:

https://github.com/exercism/nim/blob/master/exercises/react/test_react.nim
https://github.com/exercism/go/blob/master/exercises/react/react_test.go
https://github.com/exercism/fsharp/blob/master/exercises/react/ReactTests.fs
https://github.com/exercism/rust/blob/master/exercises/react/tests/react.rs

It's perfectly fine if the functions and objects are composed differently (aka, it's fine if createInput returns 2 functions, instead of an object, or createComputed takes a different value or list of values than when createInput returns objects), as long as the grant scheme of things stays similar.

It's also okay to make the executive decision to divert from the canonical data (I can make that decision), if it suits our language better.

In most reactive systems "events" are more important than "values" - otherwise the principle of least surprise regarding "events" is violated in preference to "values" (example in Svelte). So by default new values are reported even when they are equal to the previous value.

I somewhat agree with this, but in actuality, many reactive system output "distinct" events only. It's a choice, I suppose.

So for tests that require a different behavior (no event when value does not change) the computed needs to be configured that way.

We might be able to get an answer why it was decided to implement it as distinct events. @petertseng without asking you to go back and dig deep; perhaps you recall why the exercise was implemented and canonicalised as it currently still is?

@peerreynders FWIW: I had a hard time completing the JavaScript exercise because of this unexpected behaviour. I also expect events to always be "propagated", regardless of the distinctness.

@SleeplessByte
Copy link
Member Author

(sidenote: I think this way of implementing is fine, and after some consensus / more information, we can just go ahead and do it IMO).

@petertseng
Copy link
Member

Sorry, it seems I don't know the answer to that particular question - the exercise predates me. The earliest implementation is exercism/go#170, at which point the test was already present at https://github.com/exercism/go/pull/170/files#diff-fbee2dfe4416638f307806cda10cdd55eed82a650aa4518315e8a92e693f954eR108-R110. Though I would eventually go on to canonicalise it that way, I had not been considering alternative choices when canonicalising it.

@peerreynders
Copy link
Contributor

peerreynders commented Dec 20, 2020

Depending on the language being used establishing value equality in the presence of generics (parameterized types) can be tricky. It's not a problem with languages like Rust with std::cmp::PartialEq (or Haskell with Eq) but TypeScript doesn't have a standard interface for equivalence relations (likely because primitives can't implement an interface).

That is why the example implementation uses the signature:

function createComputed<T>(
  fn: ComputeFn<T>,
  value?: T,
  equal?: boolean | EqualFn<T>,
  options?: { name?: string }
): GetterFn<T> 

true for equal results in

const defaultEqual = <T>(lhs: T, rhs: T): boolean => lhs === rhs;

being used (and still NaN === NaN is always false) - otherwise a (lhs: T, rhs: T) => boolean function has to be explicitly provided.
So by default equality is not checked - which means separate events are sent even when the reported value doesn't change.

@SleeplessByte
Copy link
Member Author

@petertseng thank you.

@peerreynders yes I saw that; feels like react and preacts "shouldComponentUpdate". I think we should go ahead and implement this; see if it's possible to match all the test. If it deems impossible, because we took a different approach, set those test to false in the tests.toml.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue First-time contributors preferred help wanted
Projects
None yet
Development

No branches or pull requests

4 participants