Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Implementation of new potential skinning mechanism #3723

Merged
merged 11 commits into from
Dec 17, 2019
34 changes: 20 additions & 14 deletions .babelrc
Original file line number Diff line number Diff line change
@@ -1,20 +1,26 @@
{
"sourceMaps": "inline",
"presets": [
"react",
"es2015",
"es2016"
["@babel/preset-env", {
"targets": {
"browsers": [
"last 2 versions"
],
"node": 12
},
"modules": "commonjs"
}],
"@babel/preset-typescript",
"@babel/preset-flow",
"@babel/preset-react"
],
"plugins": [
[
"transform-builtin-extend",
{
"globals": ["Error"]
}
],
"transform-class-properties",
"transform-object-rest-spread",
"transform-runtime",
"add-module-exports",
"syntax-dynamic-import"
["@babel/plugin-proposal-decorators", { "legacy": true }],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm missing why we need legacy decorators, especially given we're only just writing the code now. Shouldn't we be using the stage 2 version?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We use legacy decorators elsewhere - it turns out we need this regardless. The modern decorators also aren't compatible with this sort of thing just yet (we need more ES6 code).

"@babel/plugin-proposal-numeric-separator",
"@babel/plugin-proposal-class-properties",
"@babel/plugin-proposal-object-rest-spread",
"@babel/plugin-transform-flow-comments",
"@babel/plugin-syntax-dynamic-import",
"@babel/plugin-transform-runtime"
]
}
146 changes: 86 additions & 60 deletions .buildkite/pipeline.yaml
Original file line number Diff line number Diff line change
@@ -1,85 +1,111 @@
steps:
- label: ":eslint: Lint"
- label: ":eslint: JS Lint"
command:
- "echo '--- Install js-sdk'"
- "./scripts/ci/install-deps.sh"
- "yarn lintwithexclusions"
- "yarn stylelint"
- "yarn lint:js"
plugins:
- docker#v3.0.1:
image: "node:10"
image: "node:12"

- label: ":chains: End-to-End Tests"
agents:
# We use a xlarge sized instance instead of the normal small ones because
# e2e tests otherwise take +-8min
queue: "xlarge"
- label: ":eslint: TS Lint"
command:
# TODO: Remove hacky chmod for BuildKite
- "echo '--- Setup'"
- "chmod +x ./scripts/ci/*.sh"
- "chmod +x ./scripts/*"
- "echo '--- Install js-sdk'"
- "./scripts/ci/install-deps.sh"
- "./scripts/ci/end-to-end-tests.sh"
- "yarn lint:ts"
plugins:
- docker#v3.0.1:
image: "matrixdotorg/riotweb-ci-e2etests-env:latest"
propagate-environment: true
image: "node:12"

- label: ":karma: Tests"
agents:
# We use a medium sized instance instead of the normal small ones because
# webpack loves to gorge itself on resources.
queue: "medium"
- label: ":eslint: Types Lint"
command:
# Install chrome
- "echo '--- Installing Chrome'"
- "wget -q -O - https://dl-ssl.google.com/linux/linux_signing_key.pub | apt-key add -"
- "sh -c 'echo \"deb [arch=amd64] http://dl.google.com/linux/chrome/deb/ stable main\" >> /etc/apt/sources.list.d/google.list'"
- "apt-get update"
- "apt-get install -y google-chrome-stable"
# Run tests
# TODO: Remove hacky chmod for BuildKite
- "chmod +x ./scripts/ci/*.sh"
- "chmod +x ./scripts/*"
- "echo '--- Installing Dependencies'"
- "echo '--- Install js-sdk'"
- "./scripts/ci/install-deps.sh"
- "echo '+++ Running Tests'"
- "./scripts/ci/unit-tests.sh"
env:
CHROME_BIN: "/usr/bin/google-chrome-stable"
- "yarn lint:types"
plugins:
- docker#v3.0.1:
image: "node:10"
propagate-environment: true
image: "node:12"

- label: "🔧 Riot Tests"
agents:
# We use a medium sized instance instead of the normal small ones because
# webpack loves to gorge itself on resources.
queue: "medium"
- label: "🛠 Build"
command:
# Install chrome
- "echo '--- Installing Chrome'"
- "wget -q -O - https://dl-ssl.google.com/linux/linux_signing_key.pub | apt-key add -"
- "sh -c 'echo \"deb [arch=amd64] http://dl.google.com/linux/chrome/deb/ stable main\" >> /etc/apt/sources.list.d/google.list'"
- "apt-get update"
- "apt-get install -y google-chrome-stable"
# Run tests
# TODO: Remove hacky chmod for BuildKite
- "chmod +x ./scripts/ci/*.sh"
- "chmod +x ./scripts/*"
- "echo '--- Installing Dependencies'"
- "echo '--- Install js-sdk'"
- "./scripts/ci/install-deps.sh"
- "echo '+++ Running Tests'"
- "./scripts/ci/riot-unit-tests.sh"
env:
CHROME_BIN: "/usr/bin/google-chrome-stable"
- "yarn build"
plugins:
- docker#v3.0.1:
image: "node:10"
propagate-environment: true
image: "node:12"

# - label: ":chains: End-to-End Tests"
# agents:
# # We use a xlarge sized instance instead of the normal small ones because
# # e2e tests otherwise take +-8min
# queue: "xlarge"
# command:
# # TODO: Remove hacky chmod for BuildKite
# - "echo '--- Setup'"
# - "chmod +x ./scripts/ci/*.sh"
# - "chmod +x ./scripts/*"
# - "echo '--- Install js-sdk'"
# - "./scripts/ci/install-deps.sh"
# - "./scripts/ci/end-to-end-tests.sh"
# plugins:
# - docker#v3.0.1:
# image: "matrixdotorg/riotweb-ci-e2etests-env:latest"
# propagate-environment: true
#
# - label: ":karma: Tests"
# agents:
# # We use a medium sized instance instead of the normal small ones because
# # webpack loves to gorge itself on resources.
# queue: "medium"
# command:
# # Install chrome
# - "echo '--- Installing Chrome'"
# - "wget -q -O - https://dl-ssl.google.com/linux/linux_signing_key.pub | apt-key add -"
# - "sh -c 'echo \"deb [arch=amd64] http://dl.google.com/linux/chrome/deb/ stable main\" >> /etc/apt/sources.list.d/google.list'"
# - "apt-get update"
# - "apt-get install -y google-chrome-stable"
# # Run tests
# # TODO: Remove hacky chmod for BuildKite
# - "chmod +x ./scripts/ci/*.sh"
# - "chmod +x ./scripts/*"
# - "echo '--- Installing Dependencies'"
# - "./scripts/ci/install-deps.sh"
# - "echo '+++ Running Tests'"
# - "./scripts/ci/unit-tests.sh"
# env:
# CHROME_BIN: "/usr/bin/google-chrome-stable"
# plugins:
# - docker#v3.0.1:
# image: "node:10"
# propagate-environment: true
#
# - label: "🔧 Riot Tests"
# agents:
# # We use a medium sized instance instead of the normal small ones because
# # webpack loves to gorge itself on resources.
# queue: "medium"
# command:
# # Install chrome
# - "echo '--- Installing Chrome'"
# - "wget -q -O - https://dl-ssl.google.com/linux/linux_signing_key.pub | apt-key add -"
# - "sh -c 'echo \"deb [arch=amd64] http://dl.google.com/linux/chrome/deb/ stable main\" >> /etc/apt/sources.list.d/google.list'"
# - "apt-get update"
# - "apt-get install -y google-chrome-stable"
# # Run tests
# # TODO: Remove hacky chmod for BuildKite
# - "chmod +x ./scripts/ci/*.sh"
# - "chmod +x ./scripts/*"
# - "echo '--- Installing Dependencies'"
# - "./scripts/ci/install-deps.sh"
# - "echo '+++ Running Tests'"
# - "./scripts/ci/riot-unit-tests.sh"
# env:
# CHROME_BIN: "/usr/bin/google-chrome-stable"
# plugins:
# - docker#v3.0.1:
# image: "node:10"
# propagate-environment: true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when can they end to end tests come back?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are removed by a different PR. Plan is shortly.


- label: "🌐 i18n"
command:
Expand Down
1 change: 1 addition & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ module.exports = {
parserOptions: {
ecmaFeatures: {
jsx: true,
legacyDecorators: true,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to note it looks like this has been removed in babel-eslint v11 so adding it now seems nonideal.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we have to add it now otherwise eslint complaints aggressively :(

}
},
rules: {
Expand Down
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ practices that anyone working with the SDK needs to be be aware of and uphold:

* After creating a new component you must run `yarn reskindex` to regenerate
the `component-index.js` for the SDK (used in future for skinning)
<!-- TODO: Remove this once this approach to skinning is replaced -->

* The view's CSS file MUST have the same name (e.g. view/rooms/MessageTile.css).
CSS for matrix-react-sdk currently resides in
Expand Down
71 changes: 71 additions & 0 deletions docs/skinning.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
# Skinning

The react-sdk can be skinned to replace presentation components, CSS, or
other relevant parts of the SDK. Typically consumers will replace entire
components and get the ability for custom CSS as a result.

This doc isn't exhaustive on how skinning works, though it should cover
some of the more complicated parts such as component replacement.

## Loading a skin

1. Generate a `component-index.js` (preferably using the tools that the react-sdk
exposes). This can typically be done with a npm script like `"reskindex -h src/header"`.
2. In your app's entry point, add something like this code:
```javascript
import {loadSkin} from "matrix-react-sdk";
loadSkin(import("component-index").components);
// The rest of your imports go under this.
```
3. Import the remainder of the SDK and bootstrap your app.

It is extremely important that you **do not** import anything else from the
SDK prior to loading your skin as otherwise the skin might not work. Loading
the skin should be one of the first things your app does, if not the very
first thing.

Additionally, **do not** provide `loadSkin` with the react-sdk components
themselves otherwise the app might explode. The SDK is already aware of its
components and doesn't need to be told.

## Replacing components

Components that replace the react-sdk ones MUST have a `replaces` static
key on the component's class to describe which component it overrides. For
example, if your `VectorAuthPage` component is meant to replace the react-sdk
`AuthPage` component then you'd add `static replaces = 'views.auth.AuthPage';`
to the `VectorAuthPage` class.

Other than that, the skin just needs to be loaded normally as mentioned above.
Consumers of the SDK likely will not be interested in the rest of this section.

### SDK developer notes

Components in the react-sdk MUST be decorated with the `@replaceableComponent`
function. For components that can't use the decorator, they must use a
variation that provides similar functionality. The decorator gives consumers
an opportunity to load skinned components by abusing import ordering and
behaviour.

Decorators are executed at import time which is why we can abuse the import
ordering behaviour: importing `loadSkin` doesn't trigger any components to
be imported, allowing the consumer to specify a skin. When the consumer does
import a component (for example, `MatrixChat`), it starts to pull in all the
components via `import` statements. When the components get pulled in the
decorator checks with the skinned components to see if it should be replacing
the component being imported. The decorator then effectively replaces the
components when needed by specifying the skinned component as an override for
the SDK's component, which should in theory override critical functions like
`render()` and lifecycle event handlers.

The decorator also means that older usage of `getComponent()` is no longer
required because components should be replaced by the decorator. Eventually
the react-sdk should only have one usage of `getComponent()`: the decorator.

The decorator assumes that if `getComponent()` returns null that there is
no skinned version of the component and continues on using the SDK's component.
In previous versions of the SDK, the function would throw an error instead
because it also expected the skin to list the SDK's components as well, however
that is no longer possible due to the above.

In short, components should always be `import`ed.
39 changes: 0 additions & 39 deletions jenkins.sh

This file was deleted.

2 changes: 1 addition & 1 deletion karma.conf.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ var fs = require('fs');
* to build everything; however it's the easiest way to load our dependencies
* from node_modules.
*
* If you run karma in multi-run mode (with `yarn test-multi`), it will watch
* If you run karma in multi-run mode (with `yarn test:multi`), it will watch
* the tests for changes, and webpack will rebuild using a cache. This is much quicker
* than a clean rebuild.
*/
Expand Down
Loading