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

Infer a speculation rule's source from the other keys #295

Merged
merged 4 commits into from
Jan 11, 2024

Conversation

jeremyroman
Copy link
Collaborator

@jeremyroman jeremyroman commented Jan 10, 2024

Fixes #169.

Copy link

Preview:

@jeremyroman jeremyroman changed the title Infer a speculation rule's source from the other keys. Infer a speculation rule's source from the other keys Jan 10, 2024
@jeremyroman
Copy link
Collaborator Author

jeremyroman commented Jan 10, 2024

What do you think? First commit is my first instinct (if present validate it as now, just allow it to be absent). Second was your slight preference (ignore it entirely).

Honestly I think I have a slight preference for the former abstractly but don't feel super strongly (and admit it's nice not to have to test both).

This is pretty trivial and I'd hope we can use a lightweight process if this makes sense to you.

@jeremyroman
Copy link
Collaborator Author

jeremyroman commented Jan 11, 2024

One case that ignoring it would break (that our unit tests caught) -- a catch-all could previously be written {"source":"document"} with no conditions, but that's not possible if implicit "source" is the only option. At least not unless we make {} also universal, which feels excessively weird. Reverted the second commit but can discuss further.

Copy link
Collaborator

@domenic domenic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm convinced, let's infer it!

Maybe also consider explainer updates in this PR?

…where it works and the context is otherwise current
Copy link
Collaborator

@domenic domenic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love it!!

@jeremyroman jeremyroman merged commit 41cc015 into WICG:main Jan 11, 2024
2 checks passed
@jeremyroman jeremyroman deleted the implicit-source branch January 11, 2024 20:54
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.

Consider removing "source" from speculation rules objects
2 participants