Skip to content

API is confusing #571

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

Open
Qrysto opened this issue May 17, 2025 · 4 comments
Open

API is confusing #571

Qrysto opened this issue May 17, 2025 · 4 comments

Comments

@Qrysto
Copy link

Qrysto commented May 17, 2025

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.

@Qrysto Qrysto changed the title API is misleading API is confusing May 17, 2025
@Murphybro2
Copy link

I have to agree to with this. The config you provide to the driver function should be the config that object uses. I have looked into changing it, but to be honest I don't have a great deal of experience with modules and it would take me longer than I'd like. Do you have a quick fix in mind @Qrysto ?

@Murphybro2
Copy link

Murphybro2 commented May 27, 2025

If you create the driver in a function like so:

const driverObj = window.driver.js.driver();

function createDriver(config) {
	return {
		Start: function () {
			driverObj.setConfig(config);
			driverObj.drive();
		},
		Next: function () {
			driverObj.moveNext();
		}
	};
}

It will allow you to do the following:

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

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

Which is good enough for what I need right now. Closures for the win!

@Qrysto
Copy link
Author

Qrysto commented May 28, 2025

I had to work around this with a pretty similar approach - always creating the driverObj right before calling drive then just discard it after every use, instead of creating driverObj once and keep the reference for reusing. It worked for me, but of course I would be more happy if the original API is improved.

@Murphybro2
Copy link

I had to work around this with a pretty similar approach - always creating the driverObj right before calling drive then just discard it after every use, instead of creating driverObj once and keep the reference for reusing. It worked for me, but of course I would be more happy if the original API is improved.

Super. I can easily fix it when it's been compiled to an IIFE, but my workaround does the trick for now, without needing to fiddle with the source code.

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

No branches or pull requests

2 participants