fix: Added logger for react native which uses console.warn for error #360
Conversation
| * limitations under the License. | ||
| */ | ||
| var logging = require('@optimizely/js-sdk-logging'); | ||
| var browserIndex = require('./index.browser'); |
There was a problem hiding this comment.
Let's not make this entry point depend on another entry point. It should be independent.
There was a problem hiding this comment.
This means a lot of repeated code in both the entry points. is that ok?
There was a problem hiding this comment.
Yes that's OK. It's not that much repeated code, and we don't want the promise polyfill, the pending events dispatcher, or listening for the pagehide event.
2. Added ReactNative logger in optimizely-sdk package.
There was a problem hiding this comment.
Logger and entry point LGTM!
Please add unit test coverage for index.react_native.js (similar to what we have for the other entry points, for example: https://github.com/optimizely/javascript-sdk/blob/master/packages/optimizely-sdk/lib/index.node.tests.js).
| @@ -0,0 +1,38 @@ | |||
| var LogLevel = require('@optimizely/js-sdk-logging').LogLevel; | |||
There was a problem hiding this comment.
Please add the license header comment here.
There was a problem hiding this comment.
Please change this to 2019 only
| @@ -0,0 +1,462 @@ | |||
| /** | |||
| * Copyright 2016-2019, Optimizely | |||
There was a problem hiding this comment.
Please change this to 2019 only
| @@ -0,0 +1,38 @@ | |||
| var LogLevel = require('@optimizely/js-sdk-logging').LogLevel; | |||
There was a problem hiding this comment.
Please change this to 2019 only
| @@ -0,0 +1,83 @@ | |||
| /** | |||
| * Copyright 2016, Optimizely | |||
There was a problem hiding this comment.
Please change this to 2019 only
| @@ -0,0 +1,121 @@ | |||
| /** | |||
| * Copyright 2016-2017, 2019, Optimizely | |||
There was a problem hiding this comment.
Please change this to 2019 only
| assert.strictEqual(activate, 'control'); | ||
| }); | ||
|
|
||
| it('should be able to set and get a forced variation', function() { |
There was a problem hiding this comment.
Let's remove all these tests related to forced variation. This isn't relevant to the react native entry point.
Summary
React Native App shows a full screen Red Message when
console.erroris called. This appears to be distracting for the developers. Added a new logger for React Native which usesconsole.warnto log error.Test plan