search

ripe-tech / ripe-sdk

The public Javascript SDK for RIPE Core
https://www.platforme.com
Apache License 2.0
8 stars 4 forks source link

#491 Update Three.js version and RIPE-SDK's bundler #495

Closed NFSS10 closed 1 year ago

NFSS10 commented 1 year ago

Issue: https://github.com/ripe-tech/ripe-sdk/issues/491

Context: Since it's creation, RIPE-SDK has evolved in a way that it's now doing anything and everything. Its capabilities go from handling logic to being a gateway to RIPE-Core. It also handles PlatformE visual representation solutions such as PRC, CSR and others.

As it's used by a broad range of products, it also needs to be built in a way that enables compatibility with all of them. This approach can sometimes lead to some limitations on the evolution of a tool as technology will keep advancing and replacing older ones. This is one of those cases which led to all of the changes that can be seen in this PR.

This issue started to take shape when trying to update the Three.js version to the latest 150.1 version. Since version 148, the CommonJS addons were removed, meaning that they are now only available as ES6 Modules. It's also important to know that build/three.js and build/three.min.js are deprecated with version 150+ and removed with version 160 which means that from version 160 onwards, Three.js will only be available as ES6 Modules.


The changes: At this moment, RIPE-SDK doesn't support ES Modules, which led to this PR. What essentially this PR is doing is adding support for ES Modules by updating the bundler and updating the Three.js version to 150.1. All of this was made with retrocompatibility in mind, so if a project is installing the RIPE-SDK via npm, if it only supports CommonJS it will only use that entry (index.js), if it supports ES Modules, it will use the module entry which points to the generated bundle. The generated bundle is now updated to handle ES Modules thus, instead of just transpiling the code with Babel and merging everything together, it now has a extra step. It uses browserify to bundle the dependencies and then transpiles the code with babel and finally merges everything together.


Notes: The tag risky was added because, although I don't think this means a major version bump, because this is ensuring retrocompatibility and was thoroughly tested in different environments and projects to ensure that, I think that we all need to be a little more attentive to any irregular behaviour that may or may not be derived from these changes.

It's also worth noting that these changes added some other improvements and even some fixes! Below you can check more details about the changes and even some recordings of a multitude of projects running with this updated version of RIPE-SDK.

Changelog:

Fixes:

Improvements:

Misc:

Tests:

Baisc HTML page with CSR configurator:

https://user-images.githubusercontent.com/22588915/222800000-20dbc5c5-e9e3-47af-a741-5a5bdd78acb9.mp4



RIPE-White (Vue.js project):

https://user-images.githubusercontent.com/22588915/222798491-0edc4b07-bb39-4aab-9859-50a63b74c9a3.mp4



RIPE-SDK Demo (Python + Jinja):

https://user-images.githubusercontent.com/22588915/222798740-aade4ca1-81d3-4748-b536-1386e4b29c35.mp4



Tobias-Slack (Node.js):

https://user-images.githubusercontent.com/22588915/222798771-2a121690-6732-4089-9258-db6d0b409320.mp4



RIPE-Robin-Revamp (React Native):

https://user-images.githubusercontent.com/22588915/222798921-d7fb3d20-21a5-4da6-9244-7695208fe3d9.mp4

vercel[bot] commented 1 year ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated
ripe-sdk ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Mar 24, 2023 at 10:22AM (UTC)
NFSS10 commented 1 year ago

Two important notes:

1. Some of my review questions are obvious and we already talked before. I'm still asking them to put them on a record and increase the visibility of some decisions here.

2. I noticed final the compiled minimal js bundle (`ripe.min.js`) now has than doubled his size: went from 0.7 MB to 1.7 MB. I guess this is mainly becuase we're now including three.js but looking at https://cdn.jsdelivr.net/npm/three@0.150.1 it looks like three.min.js is only 0.6MB so these numbers don't add up very well. Do you have an idea as to why this is happening?

1- 👍 2- Yes, you are correct, the bundle now includes Three.js. About the size, the scaling is not linear so you can't compare it directly like that and Three.js packages has more than one bundle. But the sizes of the built bundle are correct: Master: master Updated: updated

Here you can see that three.js has multiple bundles with multiple sizes, you can see that the module one is more than 0.6MB imagem

Also it's not just that bundle that is being imported, there are other addons, like loaders, stats stuff for debug, etc

3rdvision commented 1 year ago

2- Yes, you are correct, the bundle now includes Three.js. About the size the scaling is not linear so you can't compare it directly like that and Three.js packages has more than one bundle. But the sizes of the built bundle are correct: Master: master Updated: updated

My only concern is regarding ripe.min.js (default bundle) size increase from ~0.7 MB to ~ 1.7 MB. This might not be desired by some RIPE SDK users - to have the bundle size more than double because of a module they will not be using and that could be loaded dynamically or separately.

I'm not sure which is best and how it impacts performance in PRC and CSR modes but I think it's worth investing time into checking which gives us the best results. Maybe you already did a dd on this.

Alternatives:

NFSS10 commented 1 year ago

My only concern is regarding ripe.min.js (default bundle) size increase from ~0.7 MB to ~ 1.7 MB. This might not be desired by some RIPE SDK users - to have the bundle size more than double because of a module they will not be using and that could be loaded dynamically or separately.

I'm not sure which is best and how it impacts performance in PRC and CSR modes but I think it's worth investing time into checking which gives us the best results. Maybe you already did a dd on this.

Alternatives:

* Dynamically load these heavy libraries like Three, we can decrease the default bundle to a minimum;

* Publish a different npm package like `ripe-sdk-csr`;

This in fact reduces the size of the bundle but we have to keep in mind that some things happened here too. 1- It is fixing those 9 missing files not being bundled, so that's adds up a little to the size. 2- It's fixing the bundle not actually being correct, as in, Three.js should have been included from the start in ripe.min.js, that's why RIPE-White was throwing that error.

Although I agree that the size theme should've been a talking point for the SDK, I think this size increase is not a big deal in this context and I think it shouldn't impact the decision here.

The reason I say this is because the bundles aren't being built with size in mind, not even the npm package. The SDK is not built in that way (atleast it's not showing that), we may focus on that but I think it's another topic for another day, it probaly falls on the refactor theme we already touched a few times.

In fact, the division was made because the decision of doing that division was made with a very narrow view of the SDK. The actual size of the bundle at the time with Three.js was way bigger than these 1.6MB, it was 8MB or more I think so, as the consideration of using the SDK at the time of the decision was just of the simple use case of it being imported via the cdn, dividing it like this was indeed a quick win and made sense. The issue with that is that the SDK is used in a broad range of products in different ways so we can't just do that. That's why the current problem in RIPE-White exists. Also, the Three.js size issue is was handled and it's not really any other way around it, It's already only importing what it's using so can't go lower than that for the current approach.

About the suggestions:

Dynamically load these heavy libraries like Three, we can decrease the default bundle to a minimum;

We could do that and it's a perk of the ES Modules. The problem is that CommonJS imports are static so you can't really do that dynamically. In some situations you could, like in the browser probably, but what about Node.js for example, it won't work there.

Publish a different npm package like ripe-sdk-csr;

That would be in the theme of refactioring the SDK. I think we could yes and in a way it can be bring a lot of benefits but it will also bring the discussion of what actually is the RIPE-SDK and what should/shouldn't do and/or be.

NFSS10 commented 1 year ago

Will merge this as everyone is ok with it 🙂

For reference: