-
Notifications
You must be signed in to change notification settings - Fork 83
bug: Remove execution context determination #787
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
Conversation
@zashraf1985 This PR should be merged first before 786. |
# Conflicts: # packages/optimizely-sdk/lib/index.browser.ts # packages/optimizely-sdk/lib/index.react_native.ts
import { createHttpPollingDatafileManager } from './plugins/datafile_manager/browser_http_polling_datafile_manager'; | ||
import { EXECUTION_CONTEXT_TYPE } from './utils/enums'; | ||
import { ExecutionContext } from './utils/execution_context'; | ||
import { createHttpPollingDatafileManager } from './plugins/datafile_manager/http_polling_datafile_manager'; |
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.
@zashraf1985 @ozayr-zaviar I'm worried that I may not have merged this correctly.
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.
I think this should probably be './plugins/datafile_manager/browser_http_polling_datafile_manager'
I think
import { createHttpPollingDatafileManager } from './plugins/datafile_manager/react_native_http_polling_datafile_manager'; | ||
import { EXECUTION_CONTEXT_TYPE } from './utils/enums'; | ||
import { ExecutionContext } from './utils/execution_context'; | ||
import { createHttpPollingDatafileManager } from './plugins/datafile_manager/http_polling_datafile_manager'; |
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 too might not be correct either.
Should it be ./plugins/datafile_manager/react_native_http_polling_datafile_manager
or './plugins/datafile_manager/http_polling_datafile_manager'
?
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.
I think this one should be react_native_http_polling_datafile_manager
. Grrr. Sorry I messed this merge up.
if (window) { | ||
return new BrowserRequestHandler(logger, timeout); | ||
} else if (process) { | ||
return new NodeRequestHandler(logger, timeout); | ||
} else { | ||
return null as unknown as RequestHandler; |
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 will get removed by #786
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.
Well the whole file will...
Summary
window
=== Browser andprocess
=== Node is being used for the new code pathTest plan
Issues