Skip to content

API is confusing #571

Description

@Qrysto

When I see driver.js's basic usage

const driverObj = driver(config);
driverObj.drive();

I immediately thought that the provided config would be tied to driverObj specifically. But I was misled, and ended up encountering this bug:

// Initialize driver.js in page 1
const driverObj1 = driver(config1);

// Initialize driver.js In page 2
const driverObj2 = driver(config2);

Then somewhere in page 1, when I run driverObj1.drive(), suddenly the tour configured in config2 is run instead of config1!

Turns out that the config is NOT tied to the returned driverObj but is always global, and when you call driver(config), it just set that config globally for ALL the driver objects in existence and created afterward. The returned driverObj doesn't play any role here, it's just a set of API that works on the global config and global state. It could just have been exposed from the library without having to call the driver() function.

So I think the current API is super confusing. I would suggest two options:

If you want to keep the current API - returning a driver object that's initialized with some config - then the provided config should be associated with that returned object no matter how many other driver objects you create afterward.

If you want to make the library a singleton, then driver() should be renamed to something like setConfig() to reflect what it really does, and it shouldn't return anything. Instead the API functions like drive() or highlight() should just be exported from the library, because it will use the global config anyway.

I can create a PR if this is agreed on.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions