Skip to content
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

I think preInit is broken #229

Closed
folknor opened this issue May 30, 2021 · 3 comments
Closed

I think preInit is broken #229

folknor opened this issue May 30, 2021 · 3 comments
Labels
bug Something isn't working

Comments

@folknor
Copy link

folknor commented May 30, 2021

I was just reading the code a bit for fun and I believe preInit is broken. I could be mistaken. I've not tested it.

export default (ctx) => {
  // Callback function to execute when mutations are observed
  const callback = (mutations, observer) => {
    mutations.forEach((mutation) => {
      // Check if "inputField" added to the DOM
      if (ctx.input) {
        // If yes disconnect the observer
        observer.disconnect();
        // Initialize autoComplete.js
        ctx.init();
      }
    });
  };
  // Create mutation observer instance
  const observer = new MutationObserver(callback);
  // Start observing the entire DOM until "inputField" is present
  // The entire document will be observed for all changes
  observer.observe(document, { childList: true, subtree: true });
};

if (ctx.input) will always fail, won't it? This is more like the code you want

export default (ctx) => {
  // Callback function to execute when mutations are observed
  const callback = (mutations, observer) => {
    mutations.forEach((mutation) => {
      if (mutation.type === "childList") {
        for (let node of mutation.addedNodes) {
          if (node.nodeType === 1 && node.tagName === "INPUT") {
            // XXX Also check that it matches ctx.options.selector
            observer.disconnect()
            ctx.init()
          }
        }
      }
    });
  };
  // Create mutation observer instance
  const observer = new MutationObserver(callback);
  // Start observing the entire DOM until "inputField" is present
  // The entire document will be observed for all changes
  observer.observe(document, { childList: true, subtree: true });
};

I am probably misunderstanding preInit and observe completely.

@folknor folknor added the bug Something isn't working label May 30, 2021
@TarekRaafat
Copy link
Owner

Good catch! You're totally correct.

It looks broken the way it is currently, and your code is the correct implementation of the observe functionality which is meant to watch for the input field until it's added to the DOM.

Would you like to pull a PR for this one, or should I take care of it?

Cheers, and have a nice day! :)

@folknor
Copy link
Author

folknor commented May 30, 2021

You can fix it. A few more thoughts:

mutations.forEach((mutation) => { must be changed to for (const mutation of mutations) so that you can break out of the loop when you find an element that matches.

After ctx.init() just do return, probably.

And of course implement the XXX comment (I use XXX to mark things that must be changed/fixed before release.)

@TarekRaafat
Copy link
Owner

Thanks man I'll take care of it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants