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

feat(hack): in app storage buster #6886

Merged
merged 4 commits into from
Jun 27, 2024
Merged

Conversation

tonypls
Copy link
Collaborator

@tonypls tonypls commented Jun 21, 2024

Issue

The dev process on feature flagged stuff can be quite slow to delete local storage keys

Description

This PR adds a new button to the featureflags manager (find with the ff query param, app.el...com/map?ff)

The button clears all local storage and reloads the page.

@VIKTORVAV99
Copy link
Member

This is not related to your changes directly but how do we feel about limiting these dev features to only dev environments and stripping it from production builds?

@tonypls
Copy link
Collaborator Author

tonypls commented Jun 24, 2024

This is not related to your changes directly but how do we feel about limiting these dev features to only dev environments and stripping it from production builds?

It can be nice for us to be able to test this stuff on production if we are debugging.

Are you thinking to limit users or for reducing bundle size?

@VIKTORVAV99
Copy link
Member

This is not related to your changes directly but how do we feel about limiting these dev features to only dev environments and stripping it from production builds?

It can be nice for us to be able to test this stuff on production if we are debugging.

Are you thinking to limit users or for reducing bundle size?

Mostly reducing the bundle size, but it's only a 5 kB change so far so it's fine for now, but if we add more dev tools it might be nice to limit them.

@tonypls
Copy link
Collaborator Author

tonypls commented Jun 24, 2024

This is not related to your changes directly but how do we feel about limiting these dev features to only dev environments and stripping it from production builds?

It can be nice for us to be able to test this stuff on production if we are debugging.
Are you thinking to limit users or for reducing bundle size?

Mostly reducing the bundle size, but it's only a 5 kB change so far so it's fine for now, but if we add more dev tools it might be nice to limit them.

Is there a way we could only download them if the query param is active?

@VIKTORVAV99
Copy link
Member

This is not related to your changes directly but how do we feel about limiting these dev features to only dev environments and stripping it from production builds?

It can be nice for us to be able to test this stuff on production if we are debugging.

Are you thinking to limit users or for reducing bundle size?

Mostly reducing the bundle size, but it's only a 5 kB change so far so it's fine for now, but if we add more dev tools it might be nice to limit them.

Is there a way we could only download them if the query param is active?

Maybe with a dynamic import, I can give it a try later on and if it works I can open a PR?

Copy link
Contributor

@silkeholmebonnen silkeholmebonnen left a comment

Choose a reason for hiding this comment

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

Nice work! I'm sure I will be using this feature 🙌

Comment on lines 61 to 66
<button
className="mt-2 self-end rounded bg-green-900 p-1 text-xs text-white"
onClick={handleClearLocalStorage}
>
Clear Local Storage
</button>
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we maybe increase the text size a bit? It is very small

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

🙌 bumped it up!

Copy link
Contributor

@silkeholmebonnen silkeholmebonnen left a comment

Choose a reason for hiding this comment

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

LGTM 🥳

@tonypls tonypls enabled auto-merge (squash) June 27, 2024 12:54
@tonypls tonypls merged commit bfc2838 into master Jun 27, 2024
21 checks passed
@tonypls tonypls deleted the tonyvanswet/avo-341-in-app-dev-tools branch June 27, 2024 12:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants