domElement not supported in Parcel injection sequence 🙏

[devonChurch]

Hi @joeldenning 👋

I was following the conversation in issue #46 . Specifically, your comments around using the "generic" SingleSPA mountParcel functionality as an interim solution until #46 had been resolved.

I do not believe that mountParcel currently works with the single-spa-angularjs skew of the SingleSPA architecture. Have you ever had AngularJS parcels working as a proof of concept before?

There are a few problems with the current implementation. I am unfortunately not experienced with AngularJS, but I will do my best to describe the issues as I see them 👍

Parcel domElement

The domElement property supplied as part of the parcelProps object is completely ignored.

This results in the Parcel application being bound to the "root" of the HTML shell and not inside of a framework context (like Vue).

It's a pretty easy fix. In single-spa-angularjs.js change thegetContainerEl function 🙌

Before: 😢

var element;
if (opts.domElementGetter) {
  ...

After: 🎉

var element;
if (props.domElement) {
  element = props.domElement;
} else if (opts.domElementGetter) {
 ...

☝️ The above change allows scenarios like a Vue application to load in a AngularJS parcel, GREAT! I am happy to put in a PR for this if you like 😃

---

The next issue comes when wanting to have an AngularJS application load in another AngularJS parcel.

AngularJS will throw an error like this which is based around an inability to "nest" AngularJS context inside of each other.

Some more context from the AngularJS Docs

While this is not a deal-breaker, I wonder if there is a way around this limitation. Do we have any AngularJS experts in the community that have any ideas here?

---

I am interested in next steps/ideas around this 🙏 both for the support of domElement and ingesting nested AngularJS applications successfully.

Thanks 👍

[joeldenning]

Hi @devonChurch, thanks for the detailed descriptions in this issue.

Have you ever had AngularJS parcels working as a proof of concept before?

No - what you see in #46 is the only time I've discussed it.

It's a pretty easy fix. In single-spa-angularjs.js change thegetContainerEl function 🙌
I am happy to put in a PR for this if you like 😃

Yes we should update this to support parcels. The single-spa-react implementation of chooseDomElementGetter does so - see https://github.com/single-spa/single-spa-react/blob/044fe8a5fd22cba5c6aaaaa197fac65fd65604bd/src/single-spa-react.js#L186. It would be best to copy that implementation over here.

The next issue comes when wanting to have an AngularJS application load in another AngularJS parcel.
While this is not a deal-breaker, I wonder if there is a way around this limitation. Do we have any AngularJS experts in the community that have any ideas here?

My guess is that there's a workaround for this. Generally with other frameworks this works if, as part of the parcel component implementation, you create a dom element manually and then manually append to the dom. When you do it that way, generally the framework doesn't know that you're nesting.

Modifying my code from here:

const domElement = document.createElement('div');
domElement.className = 'parcel-container'
$element[0].appendChild(domElement);

function getParcelProps() {
  return {
    domElement,
    ...scope.props
  }
}

If you get it all working, I'd very gladly review pull requests adding this to the library so that others can use it.

[devonChurch]

Great, I can look at emulating the React implementation in this repo to keep things consistent. Our team will use the fix I mentioned above as an interim solution until a PR is created/merged 😄

In regards to the nested AngularJS applications. This library (single-spa-angularjs) already creates a manually injected DOM wrapper so we should be good on that front 👌. I created my own additional wrapper element just to be sure, but the issue still occurs.

I did, however, implement the nested implementation here with success 🎉 . My proof-of-concept for this exploration is relatively simple, so we intend to investigate this solution further and test its viability in a larger scale production context.

I will keep this ticket updated as I make more progress.

Thanks 👍

[joeldenning]

Sounds great - looking forward to seeing pull requests for this

[devonChurch]

Hi @joeldenning 👋 I have put in a PR based on our discussion above ☝️ and your recommendation to refactor things towards the React implementation.