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

Feature Proposal: Remote as an Object #7

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

schirrel
Copy link
Contributor

Can be seen as a simplification of the middleware.
Provides an options to pass the remote values as an object that can:

  • add default async behavios
  • make it easer to manage the remote url for build or other dynamics, without had to deal with string concatenation
    also fixed the name on the file, sorry for that 😅

@schirrel
Copy link
Contributor Author

when you had the time, please take a loook @ScriptedAlchemy @jacob-ebey 🙏🏻

};

const dynamicRemote = (remote) => {
return `(resolve) => {
Copy link
Member

Choose a reason for hiding this comment

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

we should use webpack require.l for script loading here. i opened a PR adding webpack runtime requirements to promise based remotes

Copy link
Member

@ScriptedAlchemy ScriptedAlchemy Aug 1, 2022

Choose a reason for hiding this comment

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

We should so this instead

 new Promise(function(resolve, reject) {
                       var __webpack_error__ = new Error();
                       if (typeof window.${
                         containerWithAtSyntax[0]
                       } !== 'undefined') return resolve();
                       var containerUrl = ${containerWithAtSyntax[1]} ? 
                       '${containerWithAtSyntax[1]}' : remoteConfig.entry
                       __webpack_require__.l(
                         containerUrl,
                         function(event) {
                           if (typeof window.${
                             containerWithAtSyntax[0]
                           } !== 'undefined') return resolve();
                           var errorType = event && (event.type === 'load' ? 'missing' : event.type);
                           var realSrc = event && event.target && event.target.src;
                           __webpack_error__.message =
                             'Loading script failed.\\n(' + errorType + ': ' + realSrc + ')';
                           __webpack_error__.name = 'ScriptExternalLoadError';
                           __webpack_error__.type = errorType;
                           __webpack_error__.request = realSrc;
                           reject(__webpack_error__);
                         },
                         '${containerWithAtSyntax[0]}',
                       );
                     })
                  ]).then(function(){
                    res(window.${containerWithAtSyntax[0]})
                  });

@ScriptedAlchemy
Copy link
Member

Im unsure why you want to add an async: true?

Is this purely so that the promise string can be hidden from the user and instead we create the factory for returning promise new promise?

Just want to understand what/how this would be a good method for taking away the ability to call promise new promise on your own out the box.

In its current form it looks like promise new promise just does what the @ syntax would, but as a custom promise without any ability to add additional things onto that promise new promise string. correct?

@schirrel
Copy link
Contributor Author

schirrel commented Aug 8, 2022

hey @ScriptedAlchemy the reason for async : true is basically:

    1. yeah, have a factory to abstract a code that will always be pretty much the same
    1. avoid the required load of remote entry to initialize the app shell (current, if a remote is offline the whole shell wont load, instead of only when it is used)
    1. if a remote is offline, when try to use this, avoiding throw an exception. Like if i hade a page that use something from a offline remote, current (or with the 2 item here) it throw a exception and the whole page is not loaded, instead of only dont provide a single functionality

About thewebpack require.l i will work on it and test right now

@schirrel
Copy link
Contributor Author

schirrel commented Aug 8, 2022

I've tried to use __webpack_require__.l but ir tried to request the remote on the load of the app shell/start :/

BTW the code on this PR also allow the default behavior of @ to keep being used, only changes the behavior if a object is passed

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