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

Custom dispatch function #75

Merged

Conversation

jmcountryman
Copy link
Contributor

Summary

Adds a dispatchFunction option to Provider.init.

Additional Details

For usage outside the context of iframes (e.g. a web app running in a webview), allows specifying a function to use to send messages to the Consumer instead of parent.postMessage.

@jmcountryman
Copy link
Contributor Author

cc @mhemesath @roxjcalderon

Copy link
Contributor

@mhemesath mhemesath left a comment

Choose a reason for hiding this comment

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

If we can encapsulate the postMessage specific logic into its own dispatch that would be preferred.

@@ -240,14 +249,17 @@ class Application extends EventEmitter {

if (message) {
logger.log('>> provider', this.acls, message);

const dispatch = this.dispatchFunction || parent.postMessage;
Copy link
Contributor

Choose a reason for hiding this comment

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

move this logic into the init block. It should also contain the logic to skip messages when not embedded rather than coupling that logic to custom dispatch methods.

this.dispatchFunction = dispatchFunction || (message, targetOrigin) => {
    if (window.self !== window.top) {
     parent.postMessage(message, targetOrigin);
   }
 });
``

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved: 7968afd

@@ -20,15 +20,24 @@ class Application extends EventEmitter {
* a SyntaxError exception is thrown.
* @param options.options An optional object used for App to transmit details to frame
* after App is authorized.
* @param options.dispatchFunction A function that will be used to dispatch messages instead of
Copy link
Contributor

Choose a reason for hiding this comment

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

document the expected method signature

// Dont' send messages if not embedded
if (window.self === window.top) {
// Don't send messages if not embedded, unless a custom dispatch function was provided
if (!this.dispatchFunction && window.self === window.top) {
Copy link
Contributor

Choose a reason for hiding this comment

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

skipping message sending should be an implementation detail of the dispatcher as I indicate below

@@ -40,7 +40,7 @@
"eslint-plugin-jsx-a11y": "^6.3.1",
Copy link
Contributor

Choose a reason for hiding this comment

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

A bit of an aside here, we have this version of XFC: https://github.com/cerner/xfc/pull/58/files which we have delayed integrating into our systems because of IE11's lack of includes() support.

@mhemesath do you forsee this still being an issue? Should we do work to patch that?

Copy link
Contributor

Choose a reason for hiding this comment

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

That pull request didn't introduce includes? I think we should drop support for IE11.

@@ -167,8 +192,7 @@ class Application extends EventEmitter {
*/
launch() {
if (window.self !== window.top) {
// 1: Setup listeners for all incoming communication and beforeunload
window.addEventListener('message', this.handleConsumerMessage);
// 1: Setup listener for beforeunload
window.addEventListener('beforeunload', this.unload);
Copy link
Contributor

Choose a reason for hiding this comment

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

i'd probably move this out to initialize too

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved: dab024f

@roxjcalderon roxjcalderon merged commit b3065eb into oracle-samples:master Sep 20, 2021
@jmcountryman jmcountryman deleted the custom-dispatch-function branch September 20, 2021 15:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants