Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

refactor: real-time system #2228

Merged
merged 29 commits into from
May 28, 2022
Merged

refactor: real-time system #2228

merged 29 commits into from
May 28, 2022

Conversation

anwarulislam
Copy link
Member

@anwarulislam anwarulislam commented Apr 2, 2022 •
edited

Description

WebSocket System Revamp: Migrate from Option API with JS to Script Setup on TS

Checks

  • My pull request adheres to the code style of this project
  • My code requires changes to the documentation
  • I have updated the documentation as required
  • All the tests have passed

Additional Information

WebSocket.vue component migrated to script setup in TS from JS options API

@netlify
Copy link

netlify bot commented Apr 2, 2022 •
edited

‼️ Deploy request for hoppscotch rejected.

Name Link
🔨 Latest commit 93d16d5

@lgtm-com
Copy link

lgtm-com bot commented Apr 2, 2022

This pull request introduces 8 alerts when merging 3565e48 into 8cafef4 - view on LGTM.com

new alerts:

  • 8 for Unused variable, import, function or class

@anwarulislam anwarulislam changed the title refactor: migrate websocket.vue from option api to script setup on ts refactor: websocket system revamp Apr 4, 2022
@anwarulislam anwarulislam force-pushed the refactor/websocket-system-revamp branch from 3565e48 to f6d77b0 Compare April 7, 2022 18:28
@lgtm-com
Copy link

lgtm-com bot commented Apr 7, 2022

This pull request introduces 14 alerts when merging f6d77b0 into d634828 - view on LGTM.com

new alerts:

  • 14 for Unused variable, import, function or class

@lgtm-com
Copy link

lgtm-com bot commented Apr 11, 2022

This pull request introduces 21 alerts when merging ee4e23b into 745b9f7 - view on LGTM.com

new alerts:

  • 21 for Unused variable, import, function or class

@lgtm-com
Copy link

lgtm-com bot commented Apr 11, 2022

This pull request introduces 20 alerts when merging e97d9fe into 745b9f7 - view on LGTM.com

new alerts:

  • 20 for Unused variable, import, function or class

@anwarulislam anwarulislam changed the title refactor: websocket system revamp refactor: realtime system revamp Apr 12, 2022
@lgtm-com
Copy link

lgtm-com bot commented Apr 12, 2022

This pull request introduces 20 alerts when merging 53ee661 into 745b9f7 - view on LGTM.com

new alerts:

  • 20 for Unused variable, import, function or class

@anwarulislam anwarulislam marked this pull request as ready for review April 12, 2022 19:24
@lgtm-com
Copy link

lgtm-com bot commented Apr 12, 2022

This pull request introduces 23 alerts when merging 93d16d5 into 745b9f7 - view on LGTM.com

new alerts:

  • 23 for Unused variable, import, function or class

AndrewBastin
AndrewBastin requested changes Apr 13, 2022
View reviewed changes
Copy link
Member

@AndrewBastin AndrewBastin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The whole idea behind the code separation is to separate out the UI code and the connection code to be independent, so we are trying to remove concerns like, i18n, toasts and similar stuff out of the WSSessionAdapter file into a proper position.

Also, WSSessionAdapter is a really weak name, it would be better if you would rename it to WSConnection.

packages/hoppscotch-app/helpers/realtime/WSSessionAdapter.ts Outdated Show resolved Hide resolved
packages/hoppscotch-app/helpers/realtime/WSSessionAdapter.ts Outdated Show resolved Hide resolved
@lgtm-com
Copy link

lgtm-com bot commented Apr 15, 2022

This pull request introduces 28 alerts when merging ea47903 into f1c42f2 - view on LGTM.com

new alerts:

  • 28 for Unused variable, import, function or class

@anwarulislam anwarulislam force-pushed the refactor/websocket-system-revamp branch from ea47903 to dbeffd4 Compare April 15, 2022 19:56
@lgtm-com
Copy link

lgtm-com bot commented Apr 15, 2022

This pull request introduces 28 alerts when merging dbeffd4 into f1c42f2 - view on LGTM.com

new alerts:

  • 28 for Unused variable, import, function or class

@lgtm-com
Copy link

lgtm-com bot commented Apr 16, 2022

This pull request introduces 29 alerts when merging a3198af into f1c42f2 - view on LGTM.com

new alerts:

  • 29 for Unused variable, import, function or class

AndrewBastin
AndrewBastin requested changes Apr 17, 2022
View reviewed changes
Copy link
Member

@AndrewBastin AndrewBastin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some improvements required here and there and unaddressed type errors

packages/hoppscotch-app/components/realtime/Communication.vue Outdated Show resolved Hide resolved
packages/hoppscotch-app/components/realtime/Communication.vue Outdated Show resolved Hide resolved
packages/hoppscotch-app/components/realtime/Communication.vue Outdated Show resolved Hide resolved
packages/hoppscotch-app/components/realtime/Communication.vue Outdated Show resolved Hide resolved
packages/hoppscotch-app/components/realtime/Communication.vue Outdated Show resolved Hide resolved
packages/hoppscotch-app/components/realtime/Socketio.vue Outdated Show resolved Hide resolved
packages/hoppscotch-app/components/realtime/Sse.vue Outdated Show resolved Hide resolved
packages/hoppscotch-app/components/realtime/Sse.vue Outdated Show resolved Hide resolved
packages/hoppscotch-app/components/realtime/Websocket.vue Outdated Show resolved Hide resolved
packages/hoppscotch-app/components/realtime/Websocket.vue Outdated Show resolved Hide resolved
@anwarulislam anwarulislam force-pushed the refactor/websocket-system-revamp branch from c9937af to d4be1dd Compare April 19, 2022 01:35
@lgtm-com
Copy link

lgtm-com bot commented Apr 19, 2022

This pull request introduces 30 alerts when merging d4be1dd into d1b339d - view on LGTM.com

new alerts:

  • 30 for Unused variable, import, function or class

@lgtm-com
Copy link

lgtm-com bot commented Apr 19, 2022

This pull request introduces 30 alerts when merging 984b12e into d1b339d - view on LGTM.com

new alerts:

  • 30 for Unused variable, import, function or class

@lgtm-com
Copy link

lgtm-com bot commented Apr 19, 2022

This pull request introduces 30 alerts when merging 863ff52 into d1b339d - view on LGTM.com

new alerts:

  • 30 for Unused variable, import, function or class

anwarulislam and others added 24 commits May 28, 2022 11:19
@anwarulislam @liyasthomas
refactor: non-empty body sending not allowed without connection
5d5679a
@anwarulislam @liyasthomas
refactor: button disabled while no connection established
c22b0ab
@AndrewBastin @liyasthomas
fix: linter not updating when switched from non-null to null
5980bcc
@anwarulislam @liyasthomas
fix: syntax highlighting removed for raw text
1817a5d
@anwarulislam @liyasthomas
fix: ts is optional for log and default value to 0
6b6bc61
@anwarulislam @liyasthomas
fix: replaced behaviorsubject to subject for event
9e9f117
@anwarulislam @liyasthomas
refactor: wserrormessage type added
2564310
@anwarulislam @liyasthomas
refactor: siomessage type added
982fcc2
@anwarulislam @liyasthomas
refactor: sse errorhandler improved
01c5213
@anwarulislam @liyasthomas
refactor: sio auth type
8a93764
@anwarulislam @liyasthomas
refactor: type issue resolved
f311a55
@anwarulislam @liyasthomas
refactor: mqtt error and message type
39eac8e
@anwarulislam @liyasthomas
refactor: mqtt error and message type
e703a49
@anwarulislam @liyasthomas
refactor: mqtt error and message type
9bf53dc
@anwarulislam @liyasthomas
refactor: i18n function renamed
2670350
@anwarulislam @liyasthomas
refactor: sioconnection handleerror
6aa964c
@anwarulislam @liyasthomas
refactor: mqtt handleerror
69917d0
@anwarulislam @liyasthomas
refactor: logentry prefix added
9866250
@anwarulislam @liyasthomas
fix: duplicate payload
8ebdea3
@AndrewBastin @liyasthomas
chore: use nullish coaelescing instead of or
2c6f119
@AndrewBastin @liyasthomas
refactor: use prefixes
86aa550
@AndrewBastin @liyasthomas
chore: typescript improvements in sio
ead405c
@AndrewBastin @liyasthomas
chore: minor corrections in sio and MQTTConnection
88db605
@liyasthomas
chore: hide empty sidebar in app layout
8f97aa8
@liyasthomas liyasthomas force-pushed the refactor/websocket-system-revamp branch from 9f057ad to 8f97aa8 Compare May 28, 2022 05:49
@lgtm-com
Copy link

lgtm-com bot commented May 28, 2022

This pull request introduces 30 alerts when merging 8f97aa8 into 83bdd03 - view on LGTM.com

new alerts:

  • 30 for Unused variable, import, function or class

@liyasthomas liyasthomas changed the title refactor: realtime system revamp refactor: real-time system May 28, 2022
@liyasthomas liyasthomas merged commit f6950ba into hoppscotch:main May 28, 2022
kyteinsky pushed a commit to kyteinsky/hoppscotch that referenced this pull request Jun 4, 2022
@anwarulislam @AndrewBastin
@liyasthomas @kyteinsky
refactor: real-time system (hoppscotch#2228)
a418767 
Co-authored-by: Andrew Bastin <andrewbastin.k@gmail.com>
Co-authored-by: liyasthomas <liyascthomas@gmail.com>
liyasthomas added a commit that referenced this pull request Jun 16, 2022
@anwarulislam @AndrewBastin @liyasthomas
refactor: real-time system (#2228)
0b05d22 
Co-authored-by: Andrew Bastin <andrewbastin.k@gmail.com>
Co-authored-by: liyasthomas <liyascthomas@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AndrewBastin AndrewBastin AndrewBastin requested changes

@liyasthomas liyasthomas liyasthomas approved these changes

Assignees

@anwarulislam anwarulislam

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@anwarulislam @AndrewBastin @liyasthomas