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

Allow disabling warning on exit in development #1176

Merged
merged 5 commits into from
May 6, 2020
Merged

Allow disabling warning on exit in development #1176

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e384837
Don’t warn on exit in development
merapi Apr 13, 2020
834f731
Narrow down to development only (we have production/test/development?)
merapi Apr 13, 2020
1701365
Cleaner solution
merapi Apr 13, 2020
0f08f44
Use env var
merapi Apr 27, 2020
3b4c007
Add info to docs
merapi May 6, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions assets/src/edit-story/utils/usePreventWindowUnload.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ function usePreventWindowUnload() {
},
[context]
);

return setPreventUnload;
}

export default usePreventWindowUnload;
const shouldDisablePrevent = Boolean(process.env.DISABLE_PREVENT);
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be good to document it somewhere. WDYT? Maybe under the docs folder creating an ENV_VARS.md file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added some info in 3b4c007

export default shouldDisablePrevent ? () => {} : usePreventWindowUnload;
2 changes: 1 addition & 1 deletion assets/src/edit-story/utils/useWhyDidYouUpdate.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,6 @@ function useWhyDidYouUpdate(name, props) {
});
}

const isDevelopment = process.env.NODE_ENV !== 'production';
const isDevelopment = process.env.NODE_ENV === 'development';

export default isDevelopment ? useWhyDidYouUpdate : () => {};
4 changes: 4 additions & 0 deletions docs/getting-started.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,10 @@ To get a production build, run:
npm run build:js
```

### Live reload

We don't provide it OOTB. You can setup your own solution and `DISABLE_PREVENT=1 npm run dev` will help you with unwanted `beforeunload` alert.
Copy link
Contributor

Choose a reason for hiding this comment

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

IMHO, =true/=false are more natural.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It's cast to bool in the code, so it's just the docs that'd need update.

Will do that in #1476


### Testing

#### PHP Unit Tests
Expand Down