[WIP] Convert imports and exports to ES6 #1121
Closed
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
The old `module.exports` syntax doesn't work downstream in the react-sdk or riot-web layers anymore, so this brings it up to 2019 standards. We do not use default exports anymore because they are discouraged by the ES6 community. It's not an official recommendation, but it's certainly commonplace. This also changes browser-index.js and index.js to use the `src` tree instead of `lib`. This is primarily to have IDEs autocomplete the package more easily, and because we're compiling this all in the riot-web layer it doesn't really matter. We still export `lib` and `dist/browser-matrix.js` for backwards compatibility (ie: everyone who has imported from `matrix-js-sdk/lib/whatever` or uses `<script>` tags). In a future version/PR we could drop these if we feel like it. This commit does not address the test failures that are almost certainly caused by this change. While we're here, the ancient `"use strict";` flags have been removed as well. Some files were not as easily converted. Most were simple keyword changes though there are a couple instances where the file structure is redone to accommodate ES6 exports.
turt2live
commented
Dec 17, 2019
| @@ -1,11 +1,8 @@ | |||
| { | |||
| "sourceMaps": "inline", | |||
| "sourceMaps": true, | |||
There was a problem hiding this comment.
with true we just let babel decide what is best for us. When we need inline sourcemaps we ask for them.
turt2live
commented
Dec 17, 2019
| "presets": [ | ||
| ["@babel/preset-env", { | ||
| "targets": { | ||
| "browsers": [ | ||
| "last 2 versions" | ||
| ], |
There was a problem hiding this comment.
browserify takes care of the browser for us.
turt2live
commented
Dec 17, 2019
package.json
Outdated
| "build": "yarn clean && git rev-parse HEAD > git-revision.txt && yarn build:compile && yarn build:compile-browser && yarn build:types", | ||
| "build:types": "tsc --emitDeclarationOnly", | ||
| "build:compile": "babel src -s -d lib --verbose --extensions \".ts,.js\"", | ||
| "build:compile-browser": "mkdirp dist && browserify -d browser-index.js -t [ babelify --sourceMaps=inline ] > dist/browser-matrix.js", | ||
| "build:compile-browser": "mkdirp lib/browser && browserify -d src/browser-index.js -p [ tsify -p ./tsconfig.json ] -t [ babelify --sourceMaps=inline ] -t uglifyify > lib/browser/matrix.js", |
There was a problem hiding this comment.
tsify is so we can load typescript from src. uglifify is for minification.
turt2live
commented
Dec 17, 2019
| @@ -24,9 +24,6 @@ limitations under the License. | |||
| * See also `megolm.spec.js`. | |||
| */ | |||
|
|
|||
| "use strict"; | |||
| import 'source-map-support/register'; | |||
There was a problem hiding this comment.
We don't need these imports anymore - they actually make the test's sourcemaps wrong.
|
Now that this passes, I'm going to chop it up into more sensible PRs. |
Dist is where you Distribute things.
turt2live
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This is against
travis/sourcemapsbecause it breaks everything.Changes:
lib/browserto collapse the tree a bitsrc/for more logical transpiling and IDE suggestions (many IDEs don't look at ignored directories, but will if you compile the entry point)srcinstead oflib- this is for ES6 imports/exports and future typechecking.The old
module.exportssyntax doesn't work downstream in the react-sdk or riot-web layers anymore, so this brings it up to 2019 standards.We do not use default exports anymore because they are discouraged by the ES6 community. It's not an official recommendation, but it's certainly commonplace.
This also changes browser-index.js and index.js to use the
srctree instead oflib. This is primarily to have IDEs autocomplete the package more easily, and because we're compiling this all in the riot-web layer it doesn't really matter. We still exportlibanddist/browser-matrix.jsfor backwards compatibility (ie: everyone who has imported frommatrix-js-sdk/lib/whateveror uses<script>tags). In a future version/PR we could drop these if we feel like it.This commit does not address the test failures that are almost certainly caused by this change. While we're here, the ancient
"use strict";flags have been removed as well.Some files were not as easily converted. Most were simple keyword changes though there are a couple instances where the file structure is redone to accommodate ES6 exports.