Skip to content

Avoid fetches for empty feature IDs#82

Open
schalkneethling wants to merge 1 commit into
web-platform-dx:mainfrom
schalkneethling:codex/issue-80-empty-feature-id
Open

Avoid fetches for empty feature IDs#82
schalkneethling wants to merge 1 commit into
web-platform-dx:mainfrom
schalkneethling:codex/issue-80-empty-feature-id

Conversation

@schalkneethling

Copy link
Copy Markdown

Summary

Fixes #80 by preventing <baseline-status> from requesting WebStatus API URLs when the feature id is missing, empty, or whitespace-only.

What changed

  • Normalize feature ids with trim() so values like " " are treated as empty.
  • Sync the feature-id attribute alias to featureId in willUpdate() instead of updated().
  • Guard the @lit/task fetch path so falsy normalized ids return before calling fetch().
  • Add regression tests for no feature id, empty feature-id, dynamic feature-id updates, and clearing feature-id.

Why willUpdate()

Lit already handles attributeChangedCallback() for reactive properties, so this PR does not override it.

The alias sync belongs in willUpdate() because it computes state needed by the upcoming update. This also lets @lit/task see the normalized featureId before it checks its args and decides whether to run.

The previous updated() sync happened after render, and could schedule a second update. Moving the sync earlier keeps the behavior in Lit's normal lifecycle.

Test notes

The negative fetch tests use aTimeout(0) after updateComplete to give any async Task run attempt a chance to happen before asserting that no request was made.

The dynamic positive cases use waitUntil() to wait for the observable fetch count to stabilize rather than relying on a particular browser tick.

Validation

  • npx eslint baseline-status.js test/baseline-status.test.js
  • PATH=/Users/schalkneethling/.cache/codex-runtimes/codex-primary-runtime/dependencies/node/bin:$PATH npm test

All 11 browser tests pass.

@schalkneethling

Copy link
Copy Markdown
Author

Heya @devnook - I was referred to you by @captainbrosset to ping for review here. I would also be more than happy to help with the process of triaging issues, managing dependency updates, etc., so let me know if there is a way to do this. Thanks!

@captainbrosset

Copy link
Copy Markdown
Contributor

@atopal are you, or someone who worked on this codebase in the past, able to review this PR please? I've looked at it a bit, but would be more comfortable if someone who's worked on this could review. Thanks!

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.

The component didn't check if the feature id is empty, causing useless network requests

2 participants