Skip to content

Conversation

@barak007
Copy link

@barak007 barak007 commented May 3, 2023

No description provided.

Copy link
Contributor

@idoros idoros left a comment

Choose a reason for hiding this comment

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

looks nice!

@@ -0,0 +1,96 @@
import { type Plugable, type Key, internals, PlugableInternals, Val } from './types';

const proto = {
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe inline the functions on the proto instead of proxy them?

also set the Plugable type.

set(parent, key, 'hello');

expect(res[0]).to.equal('hello');
});
Copy link
Contributor

Choose a reason for hiding this comment

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

missing tests:

  • inherited record doesn't effect the origin
  • call return value of on() to stop listening
  • listen with multiple listeners (check that removing 1 doesn't change the others)

});
});

describe('Plugable (prototype api)', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

what is this test for? seems like you test the API in the previous tests.

Copy link
Author

Choose a reason for hiding this comment

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

This is related all the other tests checking the non prototype api.
at this point I'm not sure which one is preferred WDYT?

Barak Igal added 2 commits May 4, 2023 15:21
* use getThrow
* improve set default value perf
* add more tests
* expose createKey on the api
Copy link
Contributor

@daomry daomry left a comment

Choose a reason for hiding this comment

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

How about some docs, examples etc? see other packages in the repo for the formatting

@barak007
Copy link
Author

barak007 commented Jul 2, 2023

@barak007 barak007 requested a review from daomry July 2, 2023 07:31
Copy link
Contributor

@daomry daomry left a comment

Choose a reason for hiding this comment

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

please add ts-docs comments to functions you export, this is used to generate the docs

@daomry
Copy link
Contributor

daomry commented Jul 10, 2023

@daomry https://github.com/wixplosives/core3-utils/blob/48b8079026a86de6f6e22a56afa5ef8987de078b/packages/plugable/README.md

This is very wordy but I didn't get what's it's used for. can you plz explain?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants