Add RulePatcher with module that fix incorrect interfaces #142
Conversation
|
Uh I hope I will not hurt you >.< Your current approach looks interesting and very engineered, but might overshoot a bit 馃槄 I would like to propose a much simpler solution not using AST transform but store What do you think? Do I miss anything that your approach does better? |
|
No problem I'm open to discussion dude ! But I think I totally misunderstood you yesterday. I thought you were talking about an AST-based patching system like me ahah 馃槄 However I still largely favor my solution over a manual patching system. Because, well, the solution you propose involves manual work. And that's what we're trying to avoid I think 馃構 On plugins that are more than 100 rules long, going over each of them to modify this kind of small stuff would be really long and very very boring to do. And even if there are like only 4 rules out of 100 on which you have to apply this, well, you have to go through them all to search For the moment we haven't detected a lot of patterns that we need to modify manually, but with time, there might be more. Keeping all this in mind to apply them manually on all the needed rules would be really boring to do. And especially as soon as the metadata of a rule would be updated, we'd have to go over it again. So yeah, it would be a real pain in the ass to do. And now that we have the code that allows this automation, we might as well use it, right? The other thing is that this AST system could also be used for #139 and #138. We could remove the empty comments, and also look for common types to optimize them by just making an interface. |
|
And honestly, I find the code quite simple. I don't think it's a problem to maintain it. What can be overkill is the RulePatcher container. We certainly don't need to register and delete modules at runtime |
|
Mh, not sure if we are currently thinking in two different directions. #138 should definitely solved by just patching So I don't want to patch just blank lines but cases like #141 If you like to we could discuss in person via voice chat in Discord when we both find time about that 馃檪 |
|
@Julien-R44 Please rebase and update this PR |
Closes #141
Yo
Here is my attempt at what we were discussing in this issue
So we have the
RulePatcherclass, which is just a kind of container on which we will register modules. These said modules will be applied at the end of the rule file generation. Each of these modules will have access to the AST of the generated file, and can modify it. Once all the modules have been applied, the source code is generated from this modified ast.I have implemented a first module that simply converts the mapped types and interface into a union with the different members.
And it works great for the
node/file-extension-in-importruleThe only thing that is problematic is the following:
I use @babel/generator to regenerate the source code, and :
I used the
retainLinesoptions which allows having something formatted more or less like before.But we still end up with ~50 diff files compared to the original rules which have extra spaces in some places.
Example :
But it's not a big deal in my opinion.
The other thing I miss is to display the number of patches applied in the terminal report. Everything is ready thanks to the
hasPatchedproperty, it's just missing basically the console.log. But now I'm going to bed 馃構Let me know if anything is wrong