Skip to content

Make clearing TypeDoc fields from local storage optional #2908

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

Merged
merged 3 commits into from
Mar 20, 2025

Conversation

jsmith2-coveo
Copy link
Contributor

@jsmith2-coveo jsmith2-coveo commented Mar 18, 2025

Changes


Amends the process for removing TypeDoc related fields from local storage, and makes the removal of those fields optional through a boolean parameter (defaulting to false).

Goal:
Remove only TypeDoc local storage keys when toggling disablingLocalStorage usage, rather than clearing the local storage.

Rationale Behind Changes:


#2872 introduced the option to disable local storage.

When using this option, the local storage is cleared in its entirety through localStorage.clear(). As a side effect, any strictly necessary, or otherwise categorized cookies are also deleted. This functionality should be limited to remove only TypeDoc specific fields, or in the default case not remove any of the fields.

@Gerrit0
Copy link
Collaborator

Gerrit0 commented Mar 18, 2025

I believe adding options to this to be out of scope for TypeDoc. I don't want to be responsible for deciding what is "essential". The arbitrary keys you've chosen regarding saving settings aren't the complete story for settings, and properly saving setting keys would require dynamic bundle generation.

It makes no sense to me to consider settings "essential" while considering what is collapsed not to be essential - both save the state of the page between page loads, neither is actually required for typedoc to function.

If you need this behavior to comply with some awful regulatory requirement, I would much rather have the TypeDoc global have a storage property (that meets the minimal storage requirement). TypeDoc's initialization of this global would be similar to the emitter JS for TS namespaces, so the storage could be set to a filtering implementation if desired by the end user.

Either way, the exposed API cannot be broken until 0.29, so the optional parameter would have to default to preserving existing behavior.

@Gerrit0
Copy link
Collaborator

Gerrit0 commented Mar 18, 2025

After writing that I realized that your implementation isn't filtering to treat some things as essential, but to avoid deleting non-typedoc keys... that's more reasonable, though I still don't love it. It makes me want to keep all of TypeDoc's settings in a single (or small number of) keys...

@jsmith2-coveo
Copy link
Contributor Author

jsmith2-coveo commented Mar 18, 2025

Yes, the solution was aimed at keeping non-TypeDoc instances (i.e. other keys defined as strictly necessary) that occur in local storage when disabling the local storage usage for TypeDoc.

I see what you're saying, one idea could be moving the state info to a single key with the value being an array of objects pertaining to the state of the components:

Key: _typedoc_component_state_info, Value: [{ componentId: "tsd-theme", value: "dark"}, ...]

This modification could also simplify the process of removing TypeDoc related state information from the local storage.

Alternatively, it would also suffice to remove the line for localStorage.clear() from the current implementation, allowing external users to decide if they want to keep or remove the remnants in their own usage of TypeDoc.

I'd be happy to discuss further as to what might work as an implementation.

@Gerrit0
Copy link
Collaborator

Gerrit0 commented Mar 19, 2025

After thinking about it some more, I think the right approach in the future is to move TypeDoc to a single localStorage key, but that will need to wait until 0.29 as it is a change that I know will break at least one popular plugin.

For now, would adding an additional disableWritingLocalStorage property on the global be sufficient for your use?

@jsmith2-coveo
Copy link
Contributor Author

Sounds good!

I'm not entirely sure that I follow how an additional property, disableWritingLocalStorage, on the global would stop the localCache.clear() call that occurs in the disableLocalStorage function I implemented previously. Since the disableLocalStorage function already does the job of disablingWritingLocalStorage, clearing the local storage on each call is what becomes problematic.

@Gerrit0
Copy link
Collaborator

Gerrit0 commented Mar 19, 2025

My proposal is to add an additional function which disables reading/writing to localStorage, but does not do the clear() call, users such as yourself which don't want to clear everything, can use that function instead; and in the future the clearing version can be deprecated, once typedoc only writes to a single key.

@jsmith2-coveo
Copy link
Contributor Author

I follow now! Thanks @Gerrit0!

I added the new function, and corresponding documentation.

@Gerrit0
Copy link
Collaborator

Gerrit0 commented Mar 19, 2025

Perfect! Exactly what I wanted to see, I'll merge and make a release tonight.

@jsmith2-coveo
Copy link
Contributor Author

Amazing! Thanks @Gerrit0!

@Gerrit0 Gerrit0 merged commit a49c6c8 into TypeStrong:master Mar 20, 2025
8 checks passed
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.

2 participants