mirror of
https://github.com/hoppscotch/hoppscotch.git
synced 2026-04-25 16:55:59 +03:00
[GH-ISSUE #1544] Refactoring Suggestion - Codegen generators #491
Labels
No labels
CodeDay
a11y
browser limited
bug
bug fix
cli
core
critical
design
desktop
discussion
docker
documentation
duplicate
enterprise
feature
feature
fosshack
future
good first issue
hacktoberfest
help wanted
i18n
invalid
major
minor
need information
need testing
not applicable to hoppscotch
not reproducible
pull-request
question
refactor
resolved
sandbox
self-host
spam
stale
testmu
wip
wont fix
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
starred/hoppscotch#491
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Originally created by @JayceDugan on GitHub (Mar 17, 2021).
Original GitHub issue: https://github.com/hoppscotch/hoppscotch/issues/1544
Originally assigned to: @JayceDugan on GitHub.
Overview
After reviewing the existing implementation of codegen generators present in
/helpers/codegen/generators/*.js, I'd like to gather thoughts on refactoring the existing implementations to provide the following benefits:Please find considerations and questions at the bottom of this description.
Note:
Once this issue is created I will be linking a diff comparison in the comments below of a loose example suggestion of what a refactoring might look like.
Current File Implementation
Example refactoring file
Example refactoring PR Diff
IMPORTANT - This is not to be considered a proposed solution of exactly how the refactoring would be implemented across all generators, and is more to be considered a loosely proposed idea of what a refactoring might look like of an existing implementation as a sample case.
More discussion is needed first before an exact architecture solution is proposed and additional review should be done across all generators to find common properties and opportunities for an inheritance structure, not to mention any ideas that come as a result of this discussion not previously thought of.
Proposed steps:
generator()methods to isolated functions for separation of concerns.Generatorclass for common properties (requestAllowsBody(), isBasicAuthenticationRequest(), isTokenAuthenticationRequest() etc if applicable...)Alternatives
Simply completing steps 1, 2 and 3 would improve the existing implementations for future maintenance, improved readability and reduced complexity.
Considerations & Questions
@liyasthomas commented on GitHub (Mar 17, 2021):
@JDevx97 I would totally appreciate this refactoring approach.
We have a snapshot test for all code generators: https://github.com/hoppscotch/hoppscotch/blob/main/helpers/codegen/tests/codegen.spec.js introduced in #1255
cc: @AndrewBastin
@JayceDugan commented on GitHub (Mar 17, 2021):
@liyasthomas Ah brilliant, thank you for those.
That looks promising, without having looked at all generators to confirm if there's any differences on an individual level, if that spec efficiently covers existing functionality for all generators coverage and confirmation should be quite good.
As i'm sure you guys will have better insights than me,
Suggested considerations:
@liyasthomas commented on GitHub (Mar 17, 2021):
Sticking to modular approach will be exceptionally helpful. Extracting instances of codegen out of main thread is appreciated.
So the code generation part is almost independent of other part of the application. And probably shouldn't interfere with any other functionalities.
Well, these are the places codegen is being used:
github.com/hoppscotch/hoppscotch@dc5ca76d05/components/http/CodegenModal.vuegithub.com/hoppscotch/hoppscotch@1042310038/pages/index.vue (L1136)manual testing on each language generator after implementation will be the best bet to ensure that there's no potential breaking changes.
Agree 💯
/helpers/codegen:github.com/hoppscotch/hoppscotch@dc5ca76d05/helpers/codegencodegen.jscontainsCodegenModal.vue:github.com/hoppscotch/hoppscotch@dc5ca76d05/helpers/codegen/codegen.js (L31)codegenIDandcontext:github.com/hoppscotch/hoppscotch@dc5ca76d05/helpers/codegen/codegen.js (L55)github.com/hoppscotch/hoppscotch@dc5ca76d05/helpers/codegen/codegen.js@JayceDugan commented on GitHub (Mar 18, 2021):
Hey @liyasthomas thanks for the detailed brief, much appreciated. I'll be starting tomorrow night most likely. You're welcome to mark me as the assignee if you like.
Would you at all be able to expand on this in more specifics?
@liyasthomas commented on GitHub (Mar 18, 2021):
Awesome. I was mentioning about keeping everything related to codegen inside
/helpers/codegen.@JayceDugan commented on GitHub (Mar 18, 2021):
Aah gotcha, thank you. Sounds good, I'll be taking a look at the generators as a whole and planning different structures so you can expect the first PR to come through in a few days.
In the meantime thanks again and if you have anything else you'd like me to look across feel free to tag me, more than happy to get involved even if it's just to review suggestions or provide alternative ideas.
@AndrewBastin commented on GitHub (Mar 30, 2021):
@JayceDugan thanks for the implementation. In a recent commit, we added drop-in support for TypeScript in the project. I think the Codegen system is a good candidate to support TypeScript.
It would be awesome if you could port and continue the work on TypeScript. If not, that is fine too ^_^
@JayceDugan commented on GitHub (Mar 31, 2021):
Hey @AndrewBastin,
Thanks and that's a brilliant idea, more than happy too.
Also @liyasthomas I've been meaning to get back to you, sorry work has been hectic lately. Released multiple large feature sets in the last two weeks so I've been MIA. I read your comments, including the feedback in the merges & the new branch and definitely agree.
I'll be out of everything by the end of this week and looking forward to continuing with these soon as i'm out.
@JayceDugan commented on GitHub (Apr 6, 2021):
Hey guys I'm out of things now,
@AndrewBastin Would you by any chance be able to propose a sample structure you might use for the codegens?
I've played around with typescript, played around being the accurate term and wouldn't be able to say I have extensive knowledge, so if you're able to provide any direction around how you may like things or similar structures it'd be much appreciated.
@liyasthomas commented on GitHub (Jan 23, 2022):
Refactored the current code generator implementation and released codegen v2:
github.com/hoppscotch/hoppscotch@2ec401c766