Dedicated errors about repeater HOT 12 CLOSED

Can you go a little bit more into how these errors would be used in relation to the Channel API? For instance, are you saying that calls to Channel.prototype.next should reject with these errors? Or that you should be able to throw these errors in the channel executor to modify the channel somehow?

from repeater.

I just saw an error like this one here. From my experience, it is a good habit to throw errors that can be easily handled using instanceof operator just in case a very specific error handling needs to be done.

from repeater.

Ahhhh. Sure that makes sense. Actually though, channels will never remove from a buffer when it is empty or add to a buffer when it is full, so those errors were really just for development usage. I would not be opposed to using a more descriptive error class however. I actually make sure to use Error subclasses for errors which I think will be exposed to the user. ChannelOverflowErrors for instance: https://github.com/channeljs/channel/blob/master/packages/channel/src/channel.ts#L5-L18

from repeater.

I didn't see that. I think using ts-custom-error package is a good idea anyways because writing error classes in typescript is a messy and very error-prone process (easy to forget fixing the prototype etc).

Adding ts-custom-error would make @channel/channel not dependency-free anymore so I am not sure how you feel about that. If you decide that this is fine let me know and I will gladly submit a pull request.

from repeater.

Adding ts-custom-error would make @channel/channel not dependency-free anymore

Haha yeah it might sound silly but I’d rather not add a dependency so long as we can avoid it. Also I just wish there was a global CustomError class that you could subclass without worry. Maybe we can refactor all the custom errors to use a copy-pasted fork of ts-custom-error? Maybe I should just let this dependency in. I dunno haha.

from repeater.

I've been using it in most of my projects. It works just fine, code is clean and well tested and it doesn't have any dependencies. I don't see why not tbh.

from repeater.

I’m leaning towards sure why not!

from repeater.

I think devDependencies is fine. Since i don't use Typescript, i vote for No.

from repeater.

You can't put runtime libraries into devDependencies because it would crash your program.

from repeater.

You can't put runtime libraries into devDependencies because it would crash your program.

Is this true if you bundle the dependency with rollup? We’re currently using rollup-plugin-typescript2 to bundle packages and I’m wondering if rollup can bundle devDependencies to effectively erase the dependency from the npm dependency tree.

from repeater.

I am not sure but it does not sound like a great idea tbh. Even if it worked it wouldn't make much sense to do so because it would cause ts-custom-error to get bundled multiple times if used from the main app as well.

from repeater.

See #27 (comment): I think it’s unnecessary for now. Might accept more narrowly scoped PRs for subclassing Buffer errors if there is a clear use-case but only Channels should really be consuming buffers anyways.

from repeater.