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
0009 - Expose js core modules to the window object to provide an interface to every developers #12
Merged
Merged
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
549e427
Initialize the ADR
6fc3e91
Apply review feedbacks
ae56f5a
Correct typo
033a8b7
Update and rename 0005-expose-js-modules-using-window-variable.md to …
fed0b9c
Correct typos
7a0e053
Adapt ADR for latest discussions
matks 74acbd3
Update 0009-expose-js-components-using-window-variable.md
94f0daf
Update 0009-expose-js-components-using-window-variable.md
0dac411
Update 0009-expose-js-components-using-window-variable.md
0fd39b8
Update 0009-expose-js-components-using-window-variable.md
matks a89080c
Update 0009-expose-js-components-using-window-variable.md
matks File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,61 @@ | ||
# 9. Expose js components using window variable | ||
|
||
Date: 2020-08-20 | ||
|
||
## Status | ||
|
||
Accepted | ||
|
||
## Context | ||
|
||
In order for modules to use JavaScript components from the Core, they need to import them using statements like: | ||
|
||
``` | ||
// in order to use translatable type | ||
import TranslatableInput from '../../../../../admin-dev/themes/new-theme/js/components/translatable-input'; | ||
``` | ||
|
||
This path is not robust, makes CI/CD harder, and also is not compatible with some development environments using symlinks or containers. | ||
|
||
## Decision | ||
|
||
We have decided about a system which resolves in 4 concepts: | ||
|
||
1. Reusable components in BO will be available globally through `window.prestashop` (name can still be modified in short term). | ||
|
||
All PrestaShop components will be bundled together and made available in all pages using this mean. Each controller decides which components it chooses to initialize. | ||
|
||
2. Reusable components will be available as a namespace `window.prestashop.component`. | ||
|
||
The namespace will contain classes like this `prestashop.component.SomeComponent`. If you want to get a new instance of `SomeComponent`, you call `new prestashop.component.SomeComponent(...params)` | ||
|
||
3. Reusable components will be available as initialized instances through `window.prestashop.instance`. These instances are initialized with default parameters by the `initComponents` function. | ||
|
||
4. A function `initComponents` available through `prestashop.component` is responsible for building `window.prestashop.instance`. | ||
|
||
### Why a namespace and a collection of instances | ||
|
||
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 prestashop.component.SomeComponent(...myOwnParameters)`. | ||
|
||
If you need to apply some mutation to an already initialized component, you just get the global instance: `prestashop.instance.someComponent.doSomething(...)`. | ||
|
||
|
||
## First implementation | ||
|
||
To be completed once we have validated the related PR https://github.com/PrestaShop/PrestaShop/pull/20591 | ||
|
||
## Consequences | ||
|
||
What becomes easier : | ||
|
||
- As a module developer, you'll be able to use every global javascript features of the core (if they are exposed inside the window object) | ||
- As a module developer, you're no longer forced to use hard paths breaking your dev environment or CI/CD workflow | ||
- As a core developer, you're no longer forced to import every modules you need, if they are exposed in the window object, you'll be able to use them directly | ||
- We now provide a way to avoid multiple executions of the same feature, while before, every modules was using them as they wants without caring of other modules. It means that we'll maybe avoid some side effects on some features. | ||
- Modules will be able to override core components without breaking the shop update workflow | ||
|
||
What could become harder : | ||
|
||
- It will require more knowledge about the project. Before, you needed to know that TranslatableInput exist, while you'll need to also now that they are global and don't require to be imported at all. | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
... also know that ...
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.
Nice catch 😉
Will you submit a PR or I do it ?
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.
Damn