-
Notifications
You must be signed in to change notification settings - Fork 1
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
PoC integration between browser and AMD modules #1533
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
const promise = new Promise((resolve, reject) => { | ||
Liferay.Loader.require( | ||
'liferay!frontend-js-react-web$react@16.12.0/index', | ||
resolve, | ||
reject | ||
); | ||
}); | ||
|
||
export default await promise; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a special module that would be created by the build system automagically. It provides a bridge from browser modules to AMD ones, i.e, lets There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Any idea what the cost would be to migrate our AMD modules into browser modules? Or is this just a silly thought? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In theory we would be able without touching anything in the source code. We would need a tool and some configuration files to specify where to find things. However, we would need to do a PoC for that too, because the devil is in the details, and it's not the first time (in this context) we think something is easy, then we discover it is much more complex or even impossible (see what happened with webpack federation 😞 ). |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
import holi from './js/holi.js'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Import local module holi.js synchronously |
||
import react from './_amd/react.js'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Import third party module Note that this is a trick for the PoC, but in the original source code we would have something like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It looks interesting :) Mixing amd and esm modules will not cause a race condition? Does the browser do these imports sequentially? If not, I think the amd loader can be slower(I'm guessing, yes) and the browser can load assets in esm faster and a dependency of an esm can be the currently-loading amd loader There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's no race condition because we would use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I haven't checked it, but I guess it doesn't. It can simply create the dep graph, fetch the files, and visit the graph in parallel (respecting deps, of course) instantiating and resolving modules. Much in the same way we do in AMD loader. However, I haven't done any performance test yet. And I'm not 100% sure if deploying npm packages without bundling will be performant, because I don't know if the browser is going to download the modules one by one or in parallel (using HTTP 2 or something like that). This is something we need to explore before committing to any design. |
||
|
||
holi(); | ||
document.getElementById('perico').innerHTML = `React instance: ${react}`; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
export default function holi() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This would be a regular browser module. Note that this is put inside the JAR verbatim: we don't need to build anything. |
||
alert('holi!'); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,25 +16,6 @@ | |
|
||
<%@ include file="/init.jsp" %> | ||
|
||
<liferay-ui:tabs | ||
names="Alerts,Badges,Buttons,Cards,Dropdowns,Form Elements,Icons,Labels,Links,Management Toolbars,Navigation Bars,Pagination Bars,Progress Bars,Stickers" | ||
refresh="<%= false %>" | ||
> | ||
<div id="perico"></div> | ||
|
||
<% | ||
String[] sections = {"alerts", "badges", "buttons", "cards", "dropdowns", "form_elements", "icons", "labels", "links", "management_toolbars", "navigation_bars", "pagination_bars", "progress_bars", "stickers"}; | ||
|
||
for (int i = 0; i < sections.length; i++) { | ||
%> | ||
|
||
<liferay-ui:section> | ||
<clay:container-fluid> | ||
<liferay-util:include page='<%= "/partials/" + sections[i] + ".jsp" %>' servletContext="<%= application %>" /> | ||
</clay:container-fluid> | ||
</liferay-ui:section> | ||
|
||
<% | ||
} | ||
%> | ||
|
||
</liferay-ui:tabs> | ||
<script src="/o/frontend-taglib-clay-sample-web/index.js" type="module" /> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is how you bootstrap a browser module. It is the equivalent of In the future, we could make a new tag to avoid using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
By magic you mean pointing to a valid resolved path(with version of the module and a path to /o/) like the generated NPMResolver ones? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. By magic I meant "anything we want to do with it, now or in the future". So I'm proposing to have our own taglib just for the sake of having an extra layer between the JSP and the actual I don't know if in this browser modules solution we would need to have version numbers or not 🤔 In this case, the canonical names of modules contain the Most probably, we would want to do things like:
Or, altenatively, we could have some way to configure mappings from logical package names to web context + name which are then applied when building. So, doing something like:
Nothing is yet decided, but those are the possibilities I have currently in mind... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is very interesting @izaera, I also think that having this tag would probably make things easier. Can't wait to see the final result There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Well, you will have too, because this is only a brainstorming, nothing that we have decided or planned yet 😅 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
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'm turning off any build here because I don't want babel or the bundler to rewrite anything.