-
-
Notifications
You must be signed in to change notification settings - Fork 0
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: Generate sharable link for code-explorer #24
Conversation
src/hooks/use-explorer.ts
Outdated
@@ -53,62 +54,65 @@ type ExplorerState = { | |||
|
|||
}; | |||
|
|||
const createSetter = <T extends keyof ExplorerState>( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This way is in efficient. Zustand has a better way with persist middleware to store data in localstorage. https://docs.pmnd.rs/zustand/integrations/persisting-store-data We can use that instead of manually maintaining all of these operations? my bad its already there
✅ Deploy Preview for eslint-code-explorer ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Should we wait until #30 is merged before reviewing this? |
}), | ||
persist( | ||
set => ({ | ||
tool: "ast", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Assuming we are going to be adding many more languages, does it make sense to group language-related settings together rather than having them all be separate, top-level settings?
(Note: I have no idea how any of this works, so I don't have a strong opinion, just curious.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we'll group the options based on the language. I'll update the PR accordingly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change has been made as part of this pr.
Closing this in favour of #49 |
Prerequisites checklist
What is the purpose of this pull request?
What changes did you make? (Give an overview)
Generate a shareable link for Code Explorer, similar to how ESLint Playground generates shareable links
Note: Zustand natively offers functions to update the application state and automatically map it to the URL hash.
Related Issues
Is there anything you'd like reviewers to focus on?
The changes have been implemented on top #22 this PR. Functionality-wise, it's complete; I’m now working on reducing redundant code.