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

Add config to toggle StrictMode #7035

Merged
merged 9 commits into from Nov 24, 2018

Conversation

Projects
None yet
3 participants
@SeanPrashad
Copy link
Contributor

commented Nov 23, 2018

Fixes #6694: Disable strict mode with a config flag

This PR introduces the ability to toggle React Strict mode, a tool "for highlighting potential problems in an application".

@SeanPrashad
Copy link
Contributor Author

left a comment

Reminder: Also look into existing tests to see if we need a new one to match this functionality

Show resolved Hide resolved src/core/components/Root/index.js

@SeanPrashad SeanPrashad force-pushed the SeanPrashad:issue-6694 branch 2 times, most recently from d713848 to f37d61a Nov 23, 2018

@SeanPrashad

This comment has been minimized.

Copy link
Contributor Author

commented Nov 23, 2018

Hmm.. there are 2 Assertion and Index errors for the python tests: https://travis-ci.org/mozilla/addons-frontend/jobs/458638843

@SeanPrashad SeanPrashad changed the title [WIP] Add config to disable StrictMode Add config to disable StrictMode Nov 23, 2018

@SeanPrashad SeanPrashad changed the title Add config to disable StrictMode Add config to toggle StrictMode Nov 23, 2018

@willdurand

This comment has been minimized.

Copy link
Member

commented Nov 23, 2018

@SeanPrashad when you declare a new configuration parameter, you have to "white-list" it, see the clientConfigKeys key. I think you need to add it here and also in the disco settings. Grep for enableDevTools to see where it is registered.

@willdurand willdurand self-assigned this Nov 23, 2018

SeanPrashad added some commits Nov 22, 2018

@SeanPrashad SeanPrashad force-pushed the SeanPrashad:issue-6694 branch from f37d61a to 7a143b4 Nov 23, 2018

@codecov-io

This comment has been minimized.

Copy link

commented Nov 23, 2018

Codecov Report

Merging #7035 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #7035   +/-   ##
=======================================
  Coverage   97.89%   97.89%           
=======================================
  Files         252      252           
  Lines        6887     6887           
  Branches     1273     1272    -1     
=======================================
  Hits         6742     6742           
  Misses        131      131           
  Partials       14       14
Impacted Files Coverage Δ
src/core/components/Root/index.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d93b768...743348b. Read the comment docs.

@SeanPrashad SeanPrashad force-pushed the SeanPrashad:issue-6694 branch from 7a143b4 to 711e6ec Nov 23, 2018

@SeanPrashad

This comment has been minimized.

Copy link
Contributor Author

commented Nov 23, 2018

I've added it to the whitelist section in config/default.js and config/default-disco.js but the errors are still appearing in Travis 🤔

Is there anyway for me to run those tests locally?

Show resolved Hide resolved config/dev.js Outdated
Show resolved Hide resolved config/development.js Outdated
Show resolved Hide resolved src/core/components/Root/index.js Outdated
Show resolved Hide resolved src/core/components/Root/index.js Outdated

@SeanPrashad SeanPrashad force-pushed the SeanPrashad:issue-6694 branch from dc50884 to 5f60242 Nov 23, 2018

Show resolved Hide resolved src/core/components/Root/index.js Outdated
Show resolved Hide resolved src/core/components/Root/index.js Outdated
Show resolved Hide resolved src/core/components/Root/index.js Outdated
Show resolved Hide resolved src/core/components/Root/index.js Outdated

SeanPrashad added some commits Nov 23, 2018

Show resolved Hide resolved stories/setup/Provider.js Outdated

SeanPrashad added some commits Nov 23, 2018

Revert "Correct Flow error"
This reverts commit 2c6b4b7.

@willdurand willdurand merged commit 51004e7 into mozilla:master Nov 24, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
security/snyk - package.json (Add-ons Team) No new, high severity issues
Details

@SeanPrashad SeanPrashad deleted the SeanPrashad:issue-6694 branch Nov 24, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.