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

Add plugins #75

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Add plugins #75

wants to merge 8 commits into from

Conversation

treygriffith
Copy link

This change adds a new concept of Plugins to ui-box to enable arbitrary userland processing of selectors and styles.

The plugin structure closely resembles that of glamor, which it was inspired by.

In the public API, this change exposes one new function: usePlugin.

usePlugin takes as its sole argument a function which should accept and return an object with a selector property, containing the string selector for a given ruleset, and a rules property, containing an array of rules, each with a property and value.

Plugins are executed in reverse from the order they are added (also from glamor). So if you add plugins a, b, and then c, they will be executed as c->b->a.

The motivation for this change is #74. However, given that a plugin system may open up ui-box further than desired, I'm happy to open another PR which addresses #74 specifically without a generic plugin.

Copy link
Contributor

@mshwery mshwery left a comment

Choose a reason for hiding this comment

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

This looks pretty solid. I'm curious – can we test the unhappy path too? I would love to have to a good story around preventing poorly behaving plugins from completely breaking ui-box.

package.json Outdated Show resolved Hide resolved
src/plugins.ts Outdated Show resolved Hide resolved
src/plugins.ts Show resolved Hide resolved
src/plugins.ts Outdated Show resolved Hide resolved
@treygriffith
Copy link
Author

can we test the unhappy path too? I would love to have to a good story around preventing poorly behaving plugins from completely breaking ui-box.

Did you have anything in mind? Only thing I can think of is testing that a plugin returns an object roughly in the shape that we expect. But I think the danger of a plugin system is that bad plugins can break ui-box, difficult to prevent that without severely limiting what they can do.

@mshwery
Copy link
Contributor

mshwery commented Sep 8, 2020

@treygriffith if a plugin does not return { selector: string, rules: Rule[] } how will ui-box handle that? Should we throw? Should we catch the error, console it, and move on without that plugin?

@treygriffith
Copy link
Author

@mshwery I added some handling for plugins that don't return something that at least looks like what we expect. In production, it skips the plugin and moves on. In all other environments it will throw.

Let me know what you think of that approach. It obviously doesn't handle things that look valid but aren't (e.g. nonsense selectors like $my-div).

@treygriffith
Copy link
Author

@mshwery any further thoughts on this PR?

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.

2 participants