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.

Sign up for GitHub

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

Jump to bottom

fix(middleware/devtools): add enabled option and show devtools warning only if enabled #880

Merged
merged 18 commits into from
Apr 18, 2022
Merged

fix(middleware/devtools): add enabled option and show devtools warning only if enabled #880

merged 18 commits into from
Apr 18, 2022

Conversation

Aslemammad
Copy link
Member

@Aslemammad Aslemammad commented Mar 22, 2022 •
edited by dai-shi
Loading

Resolves #878
Closes #852
Also close #874

This PR adds enabled option and, if enabled=true shows the warning and then return fn immediately! This resolved the concern described in #878.

@codesandbox-ci
Copy link

codesandbox-ci bot commented Mar 22, 2022 •
edited
Loading

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 826eb58:

Sandbox Source
React Configuration
React Typescript Configuration
React Browserify Configuration
React Snowpack Configuration
React Parcel Configuration
Next.js Configuration

@dai-shi
Copy link
Member

dai-shi commented Mar 23, 2022

Thanks for working on this.
Now I looked at the change, which is 100% what I expected, I'm thinking if we can support disabling it explicitly partially for #874 (comment). Will get back to you.

@dai-shi
Copy link
Member

dai-shi commented Mar 23, 2022

So, 4b7fc77 is an idea. We have three modes: true to enable explicitly, false to disable explicitly, and undefined to auto detect (using __DEV__).

@dai-shi dai-shi changed the title fix: remove devtools warning fix(devtools): add enabled option and show devtools warning only if enabled Mar 27, 2022
@dai-shi dai-shi changed the title fix(devtools): add enabled option and show devtools warning only if enabled fix(middleware/devtools): add enabled option and show devtools warning only if enabled Mar 27, 2022
@dai-shi dai-shi mentioned this pull request Mar 27, 2022
Merged
dai-shi
dai-shi reviewed Mar 27, 2022
View reviewed changes
src/middleware/devtools.ts Outdated Show resolved Hide resolved
@dai-shi dai-shi added this to the v4.0.0 milestone Mar 30, 2022
@Aslemammad Aslemammad mentioned this pull request Mar 30, 2022
Merged
@Aslemammad Aslemammad mentioned this pull request Apr 14, 2022
Merged
dai-shi
dai-shi approved these changes Apr 18, 2022
View reviewed changes
Copy link
Member

@dai-shi dai-shi left a comment

Choose a reason for hiding this comment

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

should be good to go.

@dai-shi dai-shi merged commit 279bfb1 into pmndrs:main Apr 18, 2022
@travjenkins travjenkins mentioned this pull request Apr 21, 2022
Merged
@young-do young-do mentioned this pull request Jan 29, 2023
Merged
3 tasks
@dcorb dcorb mentioned this pull request Jun 7, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dai-shi dai-shi dai-shi approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v4.0.0
2 participants
@Aslemammad @dai-shi