PrestaShop / ADR

Architecture Decision Records for the PrestaShop project
10 stars 15 forks source link

0009 - Expose js core modules to the window object to provide an interface to every developers #12

Closed NeOMakinG closed 4 years ago

NeOMakinG commented 4 years ago
Questions Answers
Description? We need to decide about a proper workflow of exposing core js modules to every developers using a global object such as window
Type? improvement
BC breaks? no
Deprecations? no

Vote

Maintainer Vote
@eternoendless
@PierreRambaud
@matks
@jolelievre
@Progi1984
@matthieu-rolland
@atomiix
@NeOMakinG
@sowbiba
eternoendless commented 4 years ago

Here's what we discussed:

All PrestaShop components are bundled together and its code is available in all pages. Each controller decides which components it chooses to initialize.

There's not one, but two global variables: window.PS and window.prestashop. The first one is a namespace, the second one contains objects.

The PS namespace will contain classes like this PS.Component.SomeComponent. If you want to get a new instance of SomeComponent, you call new PS.Component.SomeComponent(...params)

The prestashop object contains two things: initComponents() and components.

prestashop.initComponents() is a factory method that receives an array of component names like this: ['PS.Component.SomeComponent', 'PrestaShop.Component.SomeOtherComponent']. It will create instances of the received classnames using default parameters and will store those instances in prestashop.components. So that when you create a PS.Component.SomeComponent then you will have an instance of it available in prestashop.components.someComponent.

Since you have access to both constructors and components, developers are free to choose how to initialize and control their components.

If you don't want to initialize a given component with default parameters, you can always call new PS.Component.SomeComponent(...myOwnParameters). You can even add it to the list of global components: prestashop.components.someComponent = new PS.Component.SomeComponent(...).

If you need to apply some mutation to an already initialized component, you just get the global instance: prestashop.components.someComponent.doSomething(...).

PierreRambaud commented 4 years ago

Here's what we discussed:

All PrestaShop components are bundled together and its code is available in all pages. Each controller decides which components it chooses to initialize.

There's not one, but two global variables: window.PS and window.prestashop. The first one is a namespace, the second one contains objects.

The PS namespace will contain classes like this PS.Component.SomeComponent. If you want to get a new instance of SomeComponent, you call new PS.Component.SomeComponent(...params)

The prestashop object contains two things: initComponents() and components.

prestashop.initComponents() is a factory method that receives an array of component names like this: ['PS.Component.SomeComponent', 'PrestaShop.Component.SomeOtherComponent']. It will create instances of the received classnames using default parameters and will store those instances in prestashop.components. So that when you create a PS.Component.SomeComponent then you will have an instance of it available in prestashop.components.someComponent.

Since you have access to both constructors and components, developers are free to choose how to initialize and control their components.

If you don't want to initialize a given component with default parameters, you can always call new PS.Component.SomeComponent(...myOwnParameters). You can even add it to the list of global components: prestashop.components.someComponent = new PS.Component.SomeComponent(...).

If you need to apply some mutation to an already initialized component, you just get the global instance: prestashop.components.someComponent.doSomething(...).

We should use only one global variable, don't forget when you put data in the window object, they are registered as global, this means: window.PS = PS, so having two globals variables is a risk, a module can delete one of them :/

Why not

window.prestashop.Component.SomeComponent;
window.prestashop.components;
eternoendless commented 4 years ago

I proposed using two variables to make it clear that one is a namespace and the other a collection.

so having two globals variables is a risk, a module can delete one of them

I can't see it would be less risky to have a single variable having into account that modules can do whatever they like :)

PierreRambaud commented 4 years ago

I proposed using two variables to make it clear that one is a namespace and the other a collection.

so having two globals variables is a risk, a module can delete one of them

I can't see it would be less risky to have a single variable having into account that modules can do whatever they like :)

The more globals variables you have, the more you have to lose :trollface: it can also be confusing, PS looks outdated compared to prestashop, people can think it's a deprecated variable or regarding the ecosystem, think it's an issue :sweat_smile:

eternoendless commented 4 years ago

I wanted to make a clear difference between the two:

prestashop.Component.SomeComponent   <-- the component class
prestashop.components.someComponent  <-- the component instance

The difference is too subtle there, I'm sure people will mix them up all the time.

How about this:


prestashop.component.SomeComponent    <-- the class
prestashop.allComponents.someComponent <-- the instance

// alternatives
- prestashop.global.someComponent
- prestashop.instance.someComponent
NeOMakinG commented 4 years ago

I'm in favor of

- prestashop.component.SomeComponent
- prestashop.instance.someComponent

It's way clearer tbh

jolelievre commented 4 years ago

I also agree with @NeOMakinG naming proposition. And I would add one last modif related to this naming and the initComponents function, we could simplify it so that it is used this way:

initComponents(['TranslatableInput']);

instead of using a long namespace chain, since every component classes will be stored in window.prestashop.component anyway it will be easy to access them without some complex parsing of a namespace

This also avoid declaring the namespaced classes as global, just in the prestashop component list is enough, which also allows modules to easily add their own component

matks commented 4 years ago

Opening the voting phase 🎉

PierreRambaud commented 4 years ago

LGTM

NeOMakinG commented 4 years ago

LGTM too

jolelievre commented 4 years ago

LGTM too: sorry last modif though 😅 https://github.com/PrestaShop/ADR/pull/12/files#r479310822

matthieu-rolland commented 4 years ago

LGTM

matks commented 4 years ago

As we have reached majority, this ADR is accepted.

matks commented 4 years ago

Thank you everybody !