This repository has been archived by the owner on Sep 11, 2024. It is now read-only.
Fix sourcemaps by refactoring the build system #3839
Merged
Conversation
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
This breaks the tests
We're switching to Jest anyways, and getting these things to run is basically impossible at the moment.
They rely on a working riot-web build, which this is not.
MatrixClientPeg in particular doesn't work very well with this.
With a switch to Only One Webpack™ we need a way to help developers generate the component index without a concurrent watch task. The best way to do this is to have developers import their components, but how do they do that when we support skins? The answer in this commit is to change skinning. Skinning now expects to receive your list of overrides instead of the react-sdk+branded components. For Riot this means we send over *only* the Vector components and not Vector+react-sdk. Components can then be annotated with the `replaceComponent` decorator to have them be skinnable. The decorator must take a string with the dot path of the component because we can't reliably calculate it ourselves, sadly. The decorator does a call to `getComponent` which is where the important part of the branded components not including the react-sdk is important: if the branded app includes the react-sdk then the decorator gets executed before the skin has finished loading, leading to all kinds of fun errors. This is also why the skinner lazily loads the react-sdk components to avoid importing them too early, breaking the app. The decorator will end up receiving null for a component because of the getComponent loop mentioned: the require() call is still in progress when the decorator is called, therefore we can't error out. All usages of getComponent() within the app are safe to not need such an error (the return won't be null, and developers shouldn't use getComponent() after this commit anyways). The AuthPage, being a prominent component, has been converted to demonstrate this working. Changes to riot-web are required to have this work. The reskindex script has also been altered to reflect these skinning changes - it no longer should set the react-sdk as a parent. The eventual end goal is to get rid of `getComponent()` entirely as it'll be easily replaced by imports.
We're making an assumption here that the decorator is actually all over the app when it's not.
Implementation of new potential skinning mechanism
[BREAKING] Refactor the entire build process for babel@7 and TypeScript (chunk 1 of many)
…rix-org/matrix-react-sdk into t3chguy/jest � Conflicts: � yarn.lock
Sourcemaps: develop -> feature branch
This also magically gets rid of a ton of errors
This is needed because of jestjs/jest#6229
Probably accidentally merge-conflicted out
This doesn't fix the fact that someone called it "peg".
Jest really wants you to do things the right way.
Fix tests for sourcemaps branch
Apparently we need to do this.
Regenerate i18n for sourcemaps branch
Because the tests are run directly by node, we have to use the CommonJS module syntax. We could run the thing through babel, but then we just have another babel. Windows instructions are from experience and may not be optimized.
Enable end-to-end tests for sourcemaps (+Windows instructions)
Some of the CI stuff requires the autogenerated files to be present. Easiest way to get them all is to build the thing. Previously this was done in an install stage, but we don't do that anymore.
It was counting the wrong number of updates for reasons I don't understand.
turt2live
force-pushed
the
travis/sourcemaps
branch
from
424e85c to
90c7535
Compare
dbkr
approved these changes
Jan 14, 2020
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Review with element-hq/element-web#11843
All the commits before bf6798e have been reviewed already. This PR just needs the checkmark for bf6798e and beyond.