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

Proposal for stabilization plan #668

Closed
3 of 6 tasks
izaera opened this issue Oct 4, 2021 · 10 comments
Closed
3 of 6 tasks

Proposal for stabilization plan #668

izaera opened this issue Oct 4, 2021 · 10 comments

Comments

@izaera
Copy link
Member

izaera commented Oct 4, 2021

Issue type (mark with x)

  • πŸ€” Question
  • πŸ› Bug report
  • 🎁 Feature request
  • πŸ€·β€β™€οΈ Other

Version (mark with x)

  • 2️⃣ v2.x
  • 3️⃣ v3.x

Motivation

We currently have two versions of the bundler in this repo: v2 and v3. The former being the official one, while v3 is experimental and has only been used in one project in liferay-portal.

Bundler v3 was almost built to its MVP phase but its development was then stopped because of the release of the webpack federation feature. We tried to develop a PoC based on webpack federation to get rid of bundler v2 and v3 and almost succeeded, but then needed to revert it because it didn't fulfill all our needs (specifically we couldn't guarantee which OSGi bundle provided which npm package).

Now, we have added target platforms and @liferay/cli to the mix.

Target platforms are a new build system that couples projects with a certain release of DXP or Portal so that they can leverage the full potential of our platform (i.e: instead of making almost agnostic projects, like we did before with the yeoman generator, we make very opinionated projects that target a specific Portal/DXP version).

The advantage of using target platforms is that development and configuration is much simpler than before, because the target platform can take care of almost everything for the developer, given that it knows where the project will be deployed. The con is that you are coupling your project to Liferay and, that you won't be free to tailor your build to the last detail.

As it can be seen, we have several duplicated pieces in the puzzle that we may want to get rid of. Also, now that browser modules are already available, we may want to prepare the path to migrating our AMD loader architecture to this new standard which, apparently, could be feasible (see this spike).

Proposal

I propose to first get to a stable point where:

  1. bundler v2 is kept as the official build tool
  2. yeoman generator is discontinued in favor of @liferay/cli
  3. Everything new will be developed in the bundler v3 or target-platforms projects

Then, on top of that stable code:

  1. Design a way to test it locally
  2. Provide some inital, automated, QA tests

Once we get to this phase, we may start the new iteration phase of bundler v3 (whether it is browser modules, or any other thing we decide).

Related IFIs

@bryceosterhaus
Copy link
Member

I propose to first get to a stable point where:

  1. bundler v2 is kept as the official build tool
  2. yeoman generator is discontinued in favor of @liferay/cli
  3. Everything new will be developed in the bundler v3 or target-platforms projects

Then, on top of that stable code:

  1. Design a way to test it locally
  2. Provide some inital, automated, QA tests

Sounds great to me!

@izaera
Copy link
Member Author

izaera commented Oct 5, 2021

Forgot to mention that bundler v3 is not only used in portal, but in React fragments too πŸ™„

@izaera
Copy link
Member Author

izaera commented Oct 5, 2021

Even worse, we have this default configuration that contains platform definitions for the portal (akin to target-platforms or the global liferay-portal npmscripts.config.js file).

That was placed there as an intermediate step but, as we stopped the project, it was left there.

My plan is to move that to a target-platform instead 🀞

@izaera
Copy link
Member Author

izaera commented Oct 5, 2021

Additionally, bundler v3 does not create JAR files because we wanted to make it simple and specialized for the bundling tasks. So, we need JAR support in target-platforms as we will remove js-toolkit-scripts entirely.

izaera added a commit to izaera/liferay-frontend-projects that referenced this issue Oct 5, 2021
I have removed the deploy target as per comment
liferay#668 (comment)
so I will defer the JAR implementation to a following PR.
izaera added a commit to izaera/liferay-frontend-projects that referenced this issue Oct 5, 2021
I have removed the deploy target as per comment
liferay#668 (comment)
so I will defer the JAR implementation to a following PR.
@bryceosterhaus
Copy link
Member

Forgot to mention that bundler v3 is not only used in portal, but in React fragments too πŸ™„

How challenging would it be to move those back to v2?

@izaera
Copy link
Member Author

izaera commented Oct 7, 2021

How challenging would it be to move those back to v2?

I tried with frontend-js-recharts and it made the build ~5 times slower (from 5s to 25s, IIRC) so it doesn't seem like a good idea. We introduced bundler 3 there to check if we could improve build times because Brian was upset with how long it took to bundle JS files. But that was the only place where we tested it as I said in this comment in the devserver PR.

TL;DR: it can be reverted to bundler 2 but it will impact build times (though I'm not 100% sure if it would be so much noticeable πŸ€” ).

For fragments, I have no idea. I need to check, but I would say it might not be possible or very costly because of the way fragments are structured, IIRC. I believe it is easier if we make a single webpack bundle with the fragment files than if we need to deploy each .js file as an AMD module, and that's the reason why we decided to go with bundler 3. But I recall it vaguely, so I would need to analyze it again to make sure.

However, I'm not totally sure that it is a good idea to delete bundler 3 and go back to v2 to then make another tool that will resemble bundler v3 in so many ways. If you have a look at the broser modules + import maps architecture conceptually there is a 1-to-1 mapping to the bundler 3 model, so it looks like we can simply evolve it a bit more to achieve our goals and avoid the step back 🀷

@bryceosterhaus
Copy link
Member

TL;DR: it can be reverted to bundler 2 but it will impact build times (though I'm not 100% sure if it would be so much noticeable πŸ€” ).

Ah, I wasn't aware it was such a big decrease of build times. I didn't follow the bundler 3 progress too much, so I am fairly ignorant to it and probably need to catch up on what happened there.

it looks like we can simply evolve it a bit more to achieve our goals and avoid the step back

Yeah I have no problem evolving it, I just wasn't sure if we considered it entirely dead. I trust your experience with it though and you probably have the best idea of what next steps are there.

@izaera
Copy link
Member Author

izaera commented Oct 8, 2021

Yeah I have no problem evolving it, I just wasn't sure if we considered it entirely dead. I trust your experience with it though and you probably have the best idea of what next steps are there.

I talked about this with @nhpatt yesterday to align on this and the current status is that we will skip webpack-based bundler 3 completely. I mean, we won't delete it, but leave it as it is, being only an internal tool, not made publicly available. Then, we will put all our efforts in the browser modules thing, so that we can get to a new toolkit/bundler (call it 3, 4, or whatever we decide...) and then put bundler 2 into real maintenance, and evolve/delete the current bundler 3.

The final image should be:

  1. AMD loader is removed/deprecated
  2. Bundler 2 is removed/deprecated
  3. Bundler 3 as it is now is removed or completely rewritten
  4. A new js toolkit based on @liferay/cli that can build js projects and prepare react fragments for deployment is evolved

Does that make sense?

@izaera izaera self-assigned this Oct 8, 2021
@bryceosterhaus
Copy link
Member

Yep, that makes sense to me! Let me know if you need any specific help with this plan moving forward

@izaera
Copy link
Member Author

izaera commented Oct 22, 2021

I'm going to close this, since everything is clear now, and I don't expect more discussion in this particular issue.

@izaera izaera closed this as completed Oct 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants