feat(config-editors): Remove side effects from all config editor plugins #1317
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
* Created `Injector` class, a readonly class that can instantiate `Injectable`s * Updated the `PluginLoader`, so it can load plugins with the new way of working as well as provide a Bridge plugin for all "old fashion" plugin * Started implenenting the `BroadcastReporter` and other Reporters in the new way. Couldn't help to make some quality of life improvements: * `StrykerOptions` now have the expected required/not required properties. * Created "test-helpers" package so we can share test helpers between packages.
The TestInjector is a new way of creating your System Under Test (sut) objects. You can configure what to inject and then call `TestInjector.inject(SutClass)`
Also added some unit tests for the Injector class.
* Made the context of the Injector generic. * Made injector readonly and type safe * Moved all injector related stuff to its own package: 'typed-inject' * Updated TestInjector class to now use the new typed-inject * Updated Stryker to now use the new typed-inject * Updated Stryker-api to now use types from typed-inject
…d move implementation of functions to stryker-util
…t it in its own PR
Move all stryker plugin stuff back to the api. Talked about it with Simon. We'll allow small pieces of implentation code in the api, as long as it doesn't have any side effects
ghost
assigned 
nicojs
changed the title
simondel
previously requested changes
Feb 3, 2019
| .provideValue(commonTokens.pluginResolver, pluginLoader as PluginResolver); | ||
| configEditorInjector | ||
| .provideFactory(coreTokens.pluginCreatorConfigEditor, PluginCreator.createFactory(PluginKind.ConfigEditor)) | ||
| .injectClass(ConfigEditorApplier).edit(this.config); |
There was a problem hiding this comment.
The way the config editor is used here is quite sneaky. I would prefer if this is done in a separate method so we can name this operation like we had previously.
There was a problem hiding this comment.
I agree. I've changed it in the transpiler branch.
If we merge it here, I'll fix it with that one. I'm planning on working on Stryker full time from tomorrow for about 2 weeks, so if you agree I'll merge them in quick succession.
nicojs
dismissed
simondel’s stale review
As explained, i will fix it in a later PR
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.
Note: #1313 Needs to be merged first
Removes side effects from registering all
ConfigEditorplugins:JestConfigEditorMochaRunnerConfigEditorTypeScriptConfigEditor