Skip to content

fix: cloneObject when window.jQuery set but null/undefined #2029

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.

Sign up for GitHub

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

Jump to bottom
Merged
binrysearch merged 1 commit into from
Apr 30, 2024
Merged

fix: cloneObject when window.jQuery set but null/undefined #2029

binrysearch merged 1 commit into from
Apr 30, 2024

Conversation

@atoff
Copy link
Contributor

@atoff atoff commented Dec 2, 2023 •
edited
Loading

I encountered a problem when testing locally with intro.js whereby (for some reason) window.jQuery was set on my browser to undefined. This causes an exception in cloneObject as it tries to use the value for an instance of comparison.
image

My app doesn't use jQuery, so not sure where it is coming from, but either way I think we need to assert that window.jQuery is non-nullable before continuing to the instanceof.

I only encountered this problem when defining the steps option, as cloneObject is used to clone the input.

@vercel
Copy link

vercel bot commented Dec 2, 2023 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
intro-js ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 2, 2023 9:22pm

@kingsloi
Copy link

kingsloi commented Mar 4, 2024 •
edited
Loading

cheers @atoff! I was running into the same issue using intro.js with a project made in the TALL stack.

@jeroensijbom I see you approved this PR recently, but is there any update on when it'll be merged? I'm not able to use the intro.js without first including jQuery, which the project I'm using intro.js in doesn't use, so we're loading jQuery to not really use it, and removing it breaks it 🙏

@atoff
Copy link
Contributor Author

atoff commented Mar 4, 2024 •
edited
Loading

cheers @atoff! I was running into the same issue using intro.js with a project made in the TALL stack.

@jeroensijbom I see you approved this PR recently, but is there any update on when it'll be merged? I'm not able to use the intro.js without first including jQuery, which the project I'm using intro.js in doesn't use, so we're loading jQuery to not really use it, and removing it breaks it 🙏

Workaround I am using at the moment is including

  delete window.jQuery

when I bootstrap my application / use intro.js in a component - should work for you until this is merged and released!

@kingsloi
Copy link

kingsloi commented Mar 4, 2024 •
edited
Loading

Workaround I am using at the moment is including

  delete window.jQuery

when I bootstrap my application / use intro.js in a component - should work for you until this is merged and released!

Amazing @atoff! Worked a treat - cheers sir! 🍻

@binrysearch binrysearch merged commit cda22fc into usablica:master Apr 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@binrysearch binrysearch binrysearch approved these changes

+1 more reviewer

@jeroensijbom jeroensijbom jeroensijbom approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

bug

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@atoff @kingsloi @jeroensijbom @binrysearch