Ignore invalid before values

[jianyi-gronk]

In the current _insert implementation, invalid before values (e.g., objects or null) are ignored.

However, when before is a string (or array of strings) referencing names that do not exist in the current taps, the tap is inserted at the very beginning. This behavior seems unintended.

For example:

const hook = new SyncHook();

hook.tap("A", () => {
  console.log("A");
});

hook.tap(
  {
    name: "B",
    before: "C"
  },
  () => {
    console.log("B");
  }
);

hook.tap(
  {
    name: "C",
    before: [" "]  // invalid name
  },
  () => {
    console.log("C");
  }
);

// Current output: C B A
// Expected: A B C
hook.call();

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.97%. Comparing base (934bc55) to head (e6f6194).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #198      +/-   ##
==========================================
+ Coverage   97.96%   97.97%   +0.01%     
==========================================
  Files          13       13              
  Lines         688      692       +4     
  Branches      111      113       +2     
==========================================
+ Hits          674      678       +4     
  Misses         13       13              
  Partials        1        1              

Flag	Coverage Δ
integration	97.97% <100.00%> (+0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[alexander-akait]

Is this fix #168, right?

[jianyi-gronk]

@alexander-akait When I submitted that PR, I actually hadn't discovered this issue. I only noticed the problem that before does not ignore strings referring to taps that do not yet exist.

I just reviewed the issue, and I guess the author's intended code example was something like this:

const hook = new SyncHook();

hook.tap(
  {
    name: "A",
    before: "B"
  },
  () => {
    console.log("A");
  }
);

hook.tap(
  {
    name: "B",
    before: "C"
  },
  () => {
    console.log("B");
  }
);

hook.tap(
  {
    name: "C"
  },
  () => {
    console.log("C");
  }
);

hook.call();

In the previous implementation, this code would produce the output B A C.

The core problem the issue author wanted to address is likely to support referencing taps in before that have not yet been registered at the time of taps.

This seems to be a different issue from the one I fixed.

[alexander-akait]

Yeah, I looked deeply in the issue, it is not related, anyway thanks for the fix