Re-focus sourcemap generation #1126
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
The bulk of this is just export/import changes, though there's a couple pieces to highlight: * We no longer use default exports. This is because it's discouraged by the JS community, though not in any official capacity. * We now use `polyfillSuper` for some prototype inheritance because the tests, and sometimes webpack, break on "cannot call EncryptionAlgorithm without 'new'". It's very much a workaround, and definitely not needed when we use real classes. There is some import shuffling to help keep the imports clean - this was done by my IDE.
The earlier commit, d3ce0cb, has most of the juicy details on this. In addition to d3ce's changes, we also: * Use `TestClient` in many integration tests due to subtle behaviour changes in imports when switching to ES6. Namely the behaviour where setting the request function is less reliable in the way we did it, but `TestClient` is very reliable. * We now use the Olm loader more often to avoid having to maintain so much duplicate code. This makes the imports slightly easier to read.
Having them in `src/` helps IDEs do autocomplete a bit more nicely, and helps us not get confused about which one is referencing which. They have also been converted to TypeScript for typings to be generated.
We'll let babel decide where best to put the sourcemaps. We previously needed inline sourcemaps for browserify to work, though `babelify` takes care of this now that we use `src/` (without sourcemaps in `lib/` being inline, the transform wouldn't work). Typescript sourcemaps have also been enabled as a mental reminder that they will be exported. Babel is the only thing that uses the tsconfig for generation right now, and it appears to ignore the sourcemaps field.
dbkr
approved these changes
Dec 19, 2019
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 going to
travis/sourcemapsfor safety.This PR is built on top of #1122 - see refs/pull/1122/head...refs/pull/1126/head for the real diff
We'll let babel decide where best to put the sourcemaps. We previously needed inline sourcemaps for browserify to work, though
babelifytakes care of this now that we usesrc/(without sourcemaps inlib/being inline, the transform wouldn't work).Typescript sourcemaps have also been enabled as a mental reminder that they will be exported. Babel is the only thing that uses the tsconfig for generation right now, and it appears to ignore the sourcemaps field.