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.

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: convert ipc-renderer.ts to TypeScript #19556

Merged
merged 2 commits into from Aug 2, 2019
Merged

refactor: convert ipc-renderer.ts to TypeScript #19556

merged 2 commits into from Aug 2, 2019

Conversation

miniak
Copy link
Contributor

@miniak miniak commented Jul 31, 2019 •
edited

Description of Change

Convert ipc-renderer.ts to TypeScript. Also make the ipc binding typesafe.

Note: there is an issue with ipcRenderer.sendToAll, which is not documented. We can either remove it or document it.

Checklist

  • PR description included and stakeholders cc'd
  • npm test passes
  • PR title follows semantic commit guidelines

Release Notes

Notes: Removed undocumented ipcRenderer.sendToAll().

@electron-cation electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label Jul 31, 2019
@miniak miniak self-assigned this Jul 31, 2019
@miniak miniak force-pushed the miniak/ipc-renderer-ts branch 2 times, most recently from 6d71dab to 6beef3b Compare July 31, 2019 21:32
@miniak miniak requested a review from nornagon August 1, 2019 11:56
nornagon
nornagon approved these changes Aug 1, 2019
View reviewed changes
Copy link
Member

@nornagon nornagon left a comment

Choose a reason for hiding this comment

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

seems fine to me to remove undocumented APIs. Maybe note it in the release notes?

MarshallOfSound
MarshallOfSound reviewed Aug 1, 2019
View reviewed changes
Copy link
Member

@MarshallOfSound MarshallOfSound left a comment

Choose a reason for hiding this comment

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

I think it's ok to remove this API, it was never documented and was only ever made for internal use 🤷‍♂

typings/internal-ambient.d.ts Outdated Show resolved Hide resolved
@electron-cation electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label Aug 1, 2019
@miniak @MarshallOfSound
Update typings/internal-ambient.d.ts
e8c077b 
Co-Authored-By: Samuel Attard <sattard@slack-corp.com>
@ckerr ckerr merged commit 3011a0f into master Aug 2, 2019
@release-clerk
Copy link

release-clerk bot commented Aug 2, 2019

Release Notes Persisted

Removed undocumented ipcRenderer.sendToAll().

@miniak miniak deleted the miniak/ipc-renderer-ts branch August 5, 2019 07:50
@ConradIrwin
Copy link

@miniak We are currently using ipcRenderer.sendToAll() in our electron app at Superhuman to emulate broadcast channels.

Is there a replacement and/or can we reimplement it ourselves easily?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nornagon nornagon nornagon approved these changes

@alexeykuzmin alexeykuzmin alexeykuzmin approved these changes

@MarshallOfSound MarshallOfSound MarshallOfSound approved these changes

@codebytere codebytere Awaiting requested review from codebytere

Assignees

@miniak miniak

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@miniak @ConradIrwin @nornagon @alexeykuzmin @MarshallOfSound @ckerr