Extract to parser helpers #3843
Conversation
- rename module and file from ModuleParserHelpers to ParserHelpers - change imports and usages - rename addParsedVariable to addParsedVariableToModule to add module context again
- reduces sideeffects as parser is called outside of helper - allows better reuse
…edExpression obsolete
…e(expr.range) in dependencies to ParserHelpers
…etRange(expr.range) to ParserHelpers
| }; | ||
| }; | ||
|
|
||
| ParserHelpers.evaluateToString = function setTypeof(value) { |
| }; | ||
|
|
||
| ParserHelpers.toConstantDependency = function toConstantDependency(value) { | ||
| return function(expr) { |
There was a problem hiding this comment.
could you name these anonymous function? It better for stacktraces and profiling...
|
What still personally "annoys" me, is that the callbacks of the plugins are called with the parser as the context and then mutate the parsers state implicitly by adding the ConstDependency to it. Promises would probably make the "return true/false" impossible though? |
|
Tapable could maybe return a promise in the future but it's kind of tough. Nice work on this though. This is significantly easier to read now. |
What kind of change does this PR introduce?
refactoring
Did you add tests for your changes?
Is covered by existing tests
If relevant, link to documentation update:
N/A
Summary
While doing my refactorings to es6 i noticed that certain calls to the parser repeat themselves over several modules.
First I tried to pull them as a whole into a helper function but then realized that for now a more atomic approach should probably be more reasonable, as it removes the sideeffects and makes it reusable throughout the code.
For me the these helper functions abstract a lot of the clutter away and make the code more readable as I can see with one glimpse what is happening.
Does this PR introduce a breaking change?
No
Other information
If this approach is "ok" I would love to continue on this bit by bit.
If the PR gets to big it will introduce too many conflicts with existing PRs I guess.
Therefore if I may continue, I would try this with a bunch of smaller PRs to keep the impact on files and therefore other PRs as small as possible.