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

Merge internal options to timeline options #147304

Merged
merged 1 commit into from
Apr 12, 2022

Conversation

lramos15
Copy link
Member

This PR fixes #145983

Merges InteralOptions into TimelineOptions. The timeline options structure was entirely optional before so this seems like a safe change.

The main downside to this is now the timeline API technically has access to the cacheResult and resetCache flag despite it not being exposed in the .d.ts interface. They would of course have to probe the source code for this and since it's undocumented we make no guarantees of future compatibility.

I didn't feel comfortable removing cacheResult despite it always being passed as true as I don't know enough about how the caching works currently.

/cc @bpasero

@lramos15 lramos15 added this to the April 2022 milestone Apr 12, 2022
@lramos15 lramos15 requested a review from JacksonKearl April 12, 2022 14:25
@lramos15 lramos15 self-assigned this Apr 12, 2022
@JacksonKearl JacksonKearl merged commit 20bd040 into main Apr 12, 2022
@JacksonKearl JacksonKearl deleted the lramos15/removeInternalOptions branch April 12, 2022 18:14
@github-actions github-actions bot locked and limited conversation to collaborators May 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Timeline options feedback
2 participants