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

[Bug]: localStorage.debug is overwritten in dev-mode #1083

Closed
2 tasks done
indeyets opened this issue Oct 10, 2023 · 3 comments
Closed
2 tasks done

[Bug]: localStorage.debug is overwritten in dev-mode #1083

indeyets opened this issue Oct 10, 2023 · 3 comments
Labels
bug Something isn't working

Comments

@indeyets
Copy link
Contributor

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the latest version

Current behavior 😯

In dev-mode, localStorage.debug is overwritten with 'start*' string

Expected behavior 🤔

existing value of this variable is kept, but 'start*' is appended to it, when necessary

Steps to reproduce 🕹

Steps:

  1. set localStorage.debug to some value. for example test:*
  2. run solid-start in dev-mode, refresh the page
  3. check value of localStorage.debug

Context 🔦

This variable is used by debug() package.

Here's the relevant code: https://github.com/solidjs/solid-start/blob/main/packages/start/entry-client/mount.tsx#L13-L14

It was introduced by @nksaraf originally, as far as I can see.

I can provide the patch, but want to discuss what is the ultimate desired behaviour here. I suggest to maintain whatever is currently stored there and append start:*. I'm not sure what was the intention of using import.meta.env.DEBUG and if it needs to be supported.

Your environment 🌎

No response

@indeyets indeyets added the bug Something isn't working label Oct 10, 2023
@nksaraf
Copy link
Member

nksaraf commented Oct 10, 2023

I agree that we should append to any existing value instead of overwriting.

@indeyets
Copy link
Contributor Author

@nksaraf why did you use import.meta.env.DEBUG there? is it used by any real code or I can skip that part?

@nksaraf
Copy link
Member

nksaraf commented Oct 10, 2023

I probably added that for the use case you are describing where people would want to override.. but you can remove it. its easier to let user add their own thing and us appending to it.

indeyets added a commit to indeyets/solid-start that referenced this issue Oct 10, 2023
"start:*" would be appended if it is not already present there.

Close solidjs#1083
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants