Building & running tests

[micahjon]

I've been working on running the haunted test suite and encountered a few issues. I'm not very familiar with GNU Make or Typescript, so apologies in advance if I'm missing something obvious.

1. Might be worth clarifying that GNU Sed (not Mac Sed) is assumed by regexes in Makefile.
   https://github.com/pranksinatra/haunted/commit/070ca682d88c7fb53e4f24c34561e1171d4f0a90

2. I'm getting a Typescript error when compiling src/use-context.ts

error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead?

60     this.value = unsubscribe ? value : Context.defaultValue;
                    ~~~~~~~~~~~

3. I'm having trouble running the final test-core.js test without compiling the core.js file located top-level in the repo, e.g.

core.js: lib/*.js
    $(COMPILE) -f es -o $@ lib/core.js

Without compiling it, core.js imports lib/core.js which tries to import JS files without JS extensions, e.g.

Let me know if there's anything obvious I'm missing. Thanks!

[matthewp]

Hey, I'd be happy to move away from the ugly see stuff and Makefile if anyone can come up with a better way of doing this. Thanks!

[micahjon]

Sounds good, I'll see if I can get it all building using just rollup and npm scripts.

Also, @matthewp , would it be possible to update line 60 in src/use-context.ts so it doesn't confuse the Typescript compiler? Is there a case when unsubscribe can be falsy?

[matthewp]

Not that I can think of, sounds like a good change!

[micahjon]

Dug deeper into the falsy unsubscribe issue. Turns out that when a context consumer is used without a provider...

// test-context.js, line 50
<div id="without-provider">
    <context-consumer></context-consumer>
</div>

then neither the unsubscribe nor value properties are defined on detail, so both variables are undefined.

haunted/src/use-context.ts

Line 58 in 09cef7c

const { unsubscribe, value } = detail as ContextDetail<T>;

I can fix the TypeScript warning by making the unsubscribe property optional on the ContextDetail type, e.g.

unsubscribe?: (this: Context<T>) => void;

and falling back to null to satisfy the this._unsubscribe type, e.g.

const { unsubscribe = null, value } = detail as ContextDetail<T>;

Since the value property is a generic type, which as far as I know already encompasses undefined, I don't think we need to mess with it. It might be cleaner to make it an optional property on ContextDetail, but doing so make TypeScript think that value is either T or undefined and causes issues downstream.

Happy to submit a PR for the unsubscribe type fix if you'd like.

[micahjon]

@matthewp , just created a PR for this type issue. Should make it possible to compile from source w/out impacting any runtime behavior. Thanks for considering!
#223

[micahjon]

Most of these issues are resolved, closing for now.