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

feat: change the init() to be an explicit action triggered from consumer code #38

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

IgorSoloydenko
Copy link

@IgorSoloydenko IgorSoloydenko commented Nov 8, 2021

There is currently a couple of problems with the way the library is consumed from the client code.

1. The import of the library with import 'cerner-smart-embeddable-lib' results in an unconditional execution of CernerSmartEmbeddableLib.init(), which is not always desirable.
In a the context of our React application and its build setup it is difficult to achieve a conditional import of this library.
The proposal is to expose a single function (e.g. initializeCernerSmartEmbeddableLib()) that can be invoked from the consumer code as necessary.
Introduction of such change requires the major version bump as it's not backwards compatible.

2. Another problem is that the list of ACLs is hard coded and limited to 'https://embedded.cerner.com', 'https://embedded.sandboxcerner.com', 'https://embedded.devcerner.com'. This may not be sufficient in many cases.
The proposal is to make the init() function receive acls as a parameter (maybe with a pre-defined old value).

Looking forward to feedback.

Thank you!

@kolkheang
Copy link
Member

kolkheang commented Nov 16, 2021

Thanks @IgorSoloydenko for contributing! I think the requested changes make sense to me.

I have an idea for item 2 above that won't use the hard-coded ACLs. There is an option to use a secret value with a callback function as part of the initialization. I am thinking that a JWT could solve this with an endpoint for public key that can be accessed to verify the token in the callback function. This will require a bit of work as there is a corresponding change in the consumer side (where the iframe is created). I don't know when we will get to this.

But, your change to allow the ACLs to be passed in as part of the initialization is useful to have for now.

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.

2 participants