Skip to content

Conversation

@bvandercar-vt
Copy link
Contributor

No description provided.

@Niek
Copy link
Collaborator

Niek commented Jul 1, 2025

We can do this, but it's a big breaking change that requires a major version bump, and honestly I don't really see much added value (other than that it's cleaner, which I agree with).

@bvandercar-vt
Copy link
Contributor Author

@Niek No worries! I agree not worth a major version bump for a refactor/style change.

I may pull some of the non-breaking style-only changes in here into another PR, and then this will only have the breaking changes, and you can keep it unmerged in case you ever have another reason to make a major version change.

@regseb
Copy link
Contributor

regseb commented Jul 16, 2025

You can keep function createCursor():

/**
 * @deprecated
 */
export const createCursor = (
  page: Page,
  start: Vector = origin,
  performRandomMoves: boolean = false,
  defaultOptions: DefaultOptions = {}
): GhostCursor => new GhostCursor(page, { start, performRandomMoves, defaultOptions });

@bvandercar-vt
Copy link
Contributor Author

You can keep function createCursor():

/**
 * @deprecated
 */
export const createCursor = (
  page: Page,
  start: Vector = origin,
  performRandomMoves: boolean = false,
  defaultOptions: DefaultOptions = {}
): GhostCursor => new GhostCursor(page, { start, performRandomMoves, defaultOptions });

Good call!

I have a couple style changes in this PR that I split into other PRs first so we can make the diff look simpler in this one. @Niek

@bvandercar-vt bvandercar-vt changed the title refactor: convert to class DRAFT: refactor: convert to class Jul 29, 2025
@bvandercar-vt bvandercar-vt changed the title DRAFT: refactor: convert to class refactor: convert to class Aug 11, 2025
@bvandercar-vt
Copy link
Contributor Author

@Niek Ready for review! My recent PRs being merged has made this a lot more review-able. And now, it should be non-breaking.

@Niek Niek requested a review from Copilot August 11, 2025 04:02
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the ghost cursor implementation from a factory function pattern to a class-based pattern. The main purpose is to modernize the code structure while maintaining backward compatibility through a deprecated factory function.

Key changes:

  • Converts createCursor factory function to GhostCursor class constructor
  • Makes all cursor methods public class methods instead of object properties
  • Maintains backward compatibility with a deprecated createCursor wrapper function

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
src/spoof.ts Main refactor converting factory function to class, adding constructor with options object parameter
src/test/spoof.spec.ts Updates test to use new class constructor instead of factory function
src/debug/browser-debug.ts Updates debug code to use new class constructor with proper options structure
README.md Updates documentation to reflect new class-based API
Comments suppressed due to low confidence (1)

src/spoof.ts:646

  • The variable elem is typed as ElementHandle<Element> from getElement, but scrollIntoView method signature expects selector: string | ElementHandle. The types don't align properly - should either pass the original selector parameter or update scrollIntoView to accept ElementHandle<Element>.
      const elem = await this.getElement(selector, optionsResolved)

@Niek
Copy link
Collaborator

Niek commented Aug 12, 2025

Awesome! Let me know when you think it's ready to be merged, @bvandercar-vt

BTW we should probably auto-gen the docs (in README.md).

@bvandercar-vt
Copy link
Contributor Author

@Niek ready to go!

@Niek Niek merged commit a626a73 into Xetera:master Sep 9, 2025
1 check passed
@Niek
Copy link
Collaborator

Niek commented Sep 9, 2025

Sorry that this took forever! Merged now! Thanks again @bvandercar-vt!

@bvandercar-vt bvandercar-vt deleted the convert-to-class branch September 11, 2025 16:47
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.

3 participants