georgemandis / konami-js

Adding the Konami Code easter egg to your projects since 2009! Compatible with gestures on smartphones and tablets as well. Compatible with all front-end frameworks and vanilla JavaScript
http://konamijs.mand.is/
MIT License
959 stars 121 forks source link

Getting error: x_magnitude is not defined #61

Closed m4dd0c closed 1 week ago

m4dd0c commented 1 month ago

Describe the bug After initializing Konami. ie: `import { useEffect } from "react"; import Konami from "konami";

function App() { useEffect(() => { new Konami("https://google.com"); }, []);

return <div style={{ textAlign: "center" }}>Hello World

; }

export default App; ` getting error x_magnitude is not defined, whenever click event triggers.

To Reproduce Steps to reproduce the behavior:

  1. Initialize Konami
  2. Visit your development website
  3. Click anywhere.
  4. See error

Expected behavior Should get no error.

Screenshots konami_x_magnitude

Desktop (please complete the following information):

Smartphone (please complete the following information): I don't have Mobile :(

georgemandis commented 1 month ago

Did you close the issue because you found the problem? I had not heard of or seen this one before!

m4dd0c commented 1 month ago

I closed it because I saw repo is no more maintained. The issue is still there. I'm new to konami (actually got to know about it yesterday only). But you see monkeytype uses it and they are getting the same error. I could be wrong. However, I'm attaching Monkeytype url give it a look: https://monkeytype.com Image: [image: image.png] It could be because of initialization of these vars I guess: [image: image.png] having 'let' keyword may fix them. Thank you.

On Sat, 3 Aug 2024 at 21:26, George Mandis @.***> wrote:

Did you close the issue because you found the problem? I had not heard of or seen this one before!

— Reply to this email directly, view it on GitHub https://github.com/georgemandis/konami-js/issues/61#issuecomment-2266875481, or unsubscribe https://github.com/notifications/unsubscribe-auth/ASNNPCQGGIDIPS3XSERMFR3ZPT4TPAVCNFSM6AAAAABL5RRGPWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENRWHA3TKNBYGE . You are receiving this because you modified the open/close state.Message ID: @.***>

georgemandis commented 1 month ago

Hm. Could you try updating the images in your last comment? I'm curious and will look into it.

The package isn't maintained mostly because there's nothing more to do or add. It's historically worked on every browser going back to IE6. Sometimes software is... complete :)

I'm a little curious about this error though and if something about the way people are trying to use it doesn't play nicely.

I'll take a look (but please update those images if you can!)

m4dd0c commented 1 month ago

Yeah sure. Error: monkeytype_x_magnitude Source: image Step to reproduce;

  1. Visit Monkeytype
  2. Click anywhere
georgemandis commented 1 month ago

Thanks Manish! That was helpful.

There's a nested object in the original Konami class and the context for this referenced here looks like it got mangled in the transformed code (I'm looking at the error on Monketype).

For a comparison, you can see how the code works here without the error: https://konamijs.mand.is/

Can you tell me anything more about the way you're bundling Konami JS?

I might consider updating the code to accommodate these bundle tools if they're all making this bug (but there might be a setting in these bundler tools that fixes the problem too—the code is valid and works, but the code it's transforming it into isn't, unfortunately).

m4dd0c commented 1 month ago

I don't really get what you meant by bundling. But If it is about how I'm installing and using it then its npm. also I did try testing it locally, created vite + react project: // app.tsx useEffect(() => { const konami = new Konami("https://google.com"); }, []); and just got that error.

georgemandis commented 1 month ago

Thanks @m4dd0c—this is definitely an error! It's breaking the mobile experience when you load Konami-JS (the keyboard-based version still seems to work).

I've got it on my list to fix. I think Vite uses ESBuild under the hood and assumes more modern JavaScript than what this creaky-old project has.

I think I can tweak it to work (but still be backwards compatible). I'll have to get to it next weekend though.

Thanks for bringing it to my attention!

yaserahmady commented 2 weeks ago

Hey there! Love the library!

I have the same exact issue with Konami.js bundled with Vite in a framework-less project. Happens only on mobile obviously while on desktop works perfectly. Here's a link to a built website https://pagofresh-tour.netlify.app/ — the code is nothing special at all, some excerpts:

import Konami from "konami";

document.addEventListener("DOMContentLoaded", () => {
  const easterEgg = new Konami(() => {
    busEasterEgg();
  });
});

Dependencies' versions:

{
  "vite": "^5.3.4",
  "konami": "^1.6.3"
}
georgemandis commented 2 weeks ago

@yaserahmady @m4dd0c Thank you both for calling this to my attention! Turns out the fix was easy.

@m4dd0c Sorry—I know you reached out how contributing and I dropped the ball. I fired this up locally to see how I could best guide you, but the fix was so obvious after I did that I just decided to make the change. Sincerely appreciate the gesture though!

I tested importing this as a module locally and it worked as expected after that (back to working on touch devices!). Could one or both of you give it a look and let me know if it's working for you? If it is, I'll merge this and update the npm package as well.

My goals with this project has been to leave it as untouched as possible so that it could still boast working on nearly 20 (!!) years of browsers. I'm surprised and pleased it needed a tweak 15 years later, but am happy that the tweaks still allow for backward compatibility.

You can find the PR here:

yaserahmady commented 2 weeks ago
npm install git+https://github.com/georgemandis/konami-js#module-support-fix

Hey @georgemandis, thank you for the fix. I confirm it works perfectly for me!

georgemandis commented 2 weeks ago

Great! I'll get the main package updated soon.