Skip to content

fix: Idempotent start - #23

Closed
plajdo wants to merge 2 commits into
mainfrom
fix/idempotent-start
Closed

fix: Idempotent start#23
plajdo wants to merge 2 commits into
mainfrom
fix/idempotent-start

Conversation

@plajdo

@plajdoplajdo commented Apr 25, 2026•
edited
Loading

Copy link
Copy Markdown
Contributor

Added check whether any subscriptions for current Reactor are active. This makes start function idempotent for callers, and effectively prevents creating duplicate subscriptions.

Condition: transform function must be free of side effects, as empty transform will be called multiple times, until first subscription is created for the Reactor.

Disabled auto-start of AnyReactor type-erased type.
Previously this auto-start behaviour caused incorrectly stored Combine/non-combine subscriptions to external events from viewModels. Subscriptions were being stored under the wrapper type, which caused confusion and non-clear event routing.

Effects:

Call to viewModel.start() now has to be explicit and will always be forwarded to correct concrete Reactor type.

@plajdoplajdo self-assigned this Apr 25, 2026
@plajdoplajdo added the bug Something isn't working label Apr 25, 2026
andrej-jasso
andrej-jasso reviewed Jul 1, 2026
View reviewed changes
Comment threadREADME.md
@ViewModel var model: AnyReactor = MyViewModel().eraseToAnyReactor()
```

`AnyReactor` does not start the wrapped reactor automatically. Start it explicitly:

@andrej-jassoandrej-jassoJul 1, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This needs to go to BREAKING CHANGES section after.

@andrej-jassoandrej-jassoJul 1, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@plajdo can you please create an MD.file tracking the breaking changes for each version. Its easier to maintain if its part of the PR.

@plajdoplajdoJul 1, 2026

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

I cannot push to this branch anymore, I will create a new PR from fork. If changelog is needed, please create one separately.

However: I checked that this change should not affect any of company's projects, so no further changes are necessary (calling start manually was a recommended pattern anyway).

@andrej-jasso
andrej-jasso requested a review from CopilotJuly 1, 2026 11:10
CopilotAI reviewed Jul 1, 2026
View reviewed changes

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR aims to make Reactor.start() safe to call multiple times by avoiding duplicate external subscriptions, and changes AnyReactor lifecycle so the wrapped reactor is not auto-started (requiring explicit start() by callers).

Changes:

  • Added a subscription-existence guard to make Reactor.start() behave idempotently.
  • Disabled AnyReactor auto-start by removing base.start() from its initializer and updating its docs.
  • Updated README to instruct explicitly calling start() for AnyReactor.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

FileDescription
Sources/GoodReactor/Core/Reactor.swiftAdds guidance on transform() side effects and guards start() based on existing subscriptions.
Sources/GoodReactor/Core/Erased/AnyReactor.swiftRemoves implicit base.start() and updates lifecycle documentation for explicit starting.
README.mdDocuments explicit start() usage for AnyReactor.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment threadSources/GoodReactor/Core/Reactor.swift
Comment threadSources/GoodReactor/Core/Erased/AnyReactor.swift
Comment threadREADME.md
@plajdo

plajdo commented Jul 1, 2026•
edited
Loading

Copy link
Copy Markdown
ContributorAuthor

Cannot push to this branch anymore, let's follow the changes at #25.

@plajdoplajdo closed this Jul 1, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@andrej-jassoandrej-jassoandrej-jasso left review comments
Copilot code reviewCopilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@MatusMistrikMatusMistrikAwaiting requested review from MatusMistrik

Assignees

@plajdoplajdo

Labels

bugSomething isn't working

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@plajdo@andrej-jasso
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all 
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
fix: Idempotent start by plajdo · Pull Request #23 · GoodRequest/GoodReactor · GitHub
Skip to content
fix: Idempotent start - #23
Closed
plajdo wants to merge 2 commits into
mainfrom 
fix/idempotent-start
Closed
fix: Idempotent start#23
plajdo wants to merge 2 commits into
mainfrom 
fix/idempotent-start
Conversation
@plajdo
@plajdoplajdo
commented
Apr 25, 2026•

edited

Loading

Copy link


Copy Markdown

Contributor
Added check whether any subscriptions for current Reactor are active. This makes start function idempotent for callers, and effectively prevents creating duplicate subscriptions.
Condition: transform function must be free of side effects, as empty transform will be called multiple times, until first subscription is created for the Reactor.
Disabled auto-start of AnyReactor type-erased type.

Previously this auto-start behaviour caused incorrectly stored Combine/non-combine subscriptions to external events from viewModels. Subscriptions were being stored under the wrapper type, which caused confusion and non-clear event routing.
Effects:
Call to viewModel.start() now has to be explicit and will always be forwarded to correct concrete Reactor type.
@plajdoplajdo
self-assigned this
Apr 25, 2026
@plajdoplajdo
added
the 
bug
Something isn't working
label
Apr 25, 2026
andrej-jasso
andrej-jasso
reviewed
Jul 1, 2026
View reviewed changes
Comment threadREADME.md
@ViewModel var model: AnyReactor = MyViewModel().eraseToAnyReactor()
```

`AnyReactor` does not start the wrapped reactor automatically. Start it explicitly:
@andrej-jassoandrej-jassoJul 1, 2026

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

This needs to go to BREAKING CHANGES section after.
@andrej-jassoandrej-jassoJul 1, 2026

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

@plajdo can you please create an MD.file tracking the breaking changes for each version. Its easier to maintain if its part of the PR.
@plajdoplajdoJul 1, 2026

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

I cannot push to this branch anymore, I will create a new PR from fork. If changelog is needed, please create one separately.
However: I checked that this change should not affect any of company's projects, so no further changes are necessary (calling start manually was a recommended pattern anyway).
@andrej-jasso
andrej-jasso
requested a review
from CopilotJuly 1, 2026 11:10
CopilotAI
reviewed
Jul 1, 2026
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview
This PR aims to make Reactor.start() safe to call multiple times by avoiding duplicate external subscriptions, and changes AnyReactor lifecycle so the wrapped reactor is not auto-started (requiring explicit start() by callers).
Changes:
  • Added a subscription-existence guard to make Reactor.start() behave idempotently.
  • Disabled AnyReactor auto-start by removing base.start() from its initializer and updating its docs.
  • Updated README to instruct explicitly calling start() for AnyReactor.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
FileDescription
Sources/GoodReactor/Core/Reactor.swiftAdds guidance on transform() side effects and guards start() based on existing subscriptions.
Sources/GoodReactor/Core/Erased/AnyReactor.swiftRemoves implicit base.start() and updates lifecycle documentation for explicit starting.
README.mdDocuments explicit start() usage for AnyReactor.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment threadSources/GoodReactor/Core/Reactor.swift
Comment threadSources/GoodReactor/Core/Erased/AnyReactor.swift
Comment threadREADME.md
@plajdo
plajdo
commented
Jul 1, 2026•

edited

Loading

Copy link


Copy Markdown

ContributorAuthor
Cannot push to this branch anymore, let's follow the changes at #25.
@plajdoplajdo
closed this
Jul 1, 2026
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@andrej-jassoandrej-jassoandrej-jasso left review comments
Copilot code reviewCopilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@MatusMistrikMatusMistrikAwaiting requested review from MatusMistrik

Assignees

@plajdoplajdo

Labels

bugSomething isn't working

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

3 participants

@plajdo@andrej-jasso

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Force GitHub README to respect dark mode
(function() {
var style = document.createElement('style');
style.textContent = '
.markdown-body {
color-scheme: dark light;
}
.markdown-body pre { background: #161b22 !important; }
.markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }
.markdown-body table th, .markdown-body table td { border-color: #30363d !important; }
.markdown-body img { background: #0d1117; }
.markdown-body blockquote { border-left-color: #8b949e; }
.markdown-body hr { border-color: #30363d; }
';
document.head.appendChild(style);
})();
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
fix: Idempotent start by plajdo · Pull Request #23 · GoodRequest/GoodReactor · GitHub
Skip to content
fix: Idempotent start - #23
Closed
plajdo wants to merge 2 commits into
mainfrom 
fix/idempotent-start
Closed
fix: Idempotent start#23
plajdo wants to merge 2 commits into
mainfrom 
fix/idempotent-start
Conversation
@plajdo
@plajdoplajdo
commented
Apr 25, 2026•

edited

Loading

Copy link


Copy Markdown

Contributor
Added check whether any subscriptions for current Reactor are active. This makes start function idempotent for callers, and effectively prevents creating duplicate subscriptions.
Condition: transform function must be free of side effects, as empty transform will be called multiple times, until first subscription is created for the Reactor.
Disabled auto-start of AnyReactor type-erased type.

Previously this auto-start behaviour caused incorrectly stored Combine/non-combine subscriptions to external events from viewModels. Subscriptions were being stored under the wrapper type, which caused confusion and non-clear event routing.
Effects:
Call to viewModel.start() now has to be explicit and will always be forwarded to correct concrete Reactor type.
@plajdoplajdo
self-assigned this
Apr 25, 2026
@plajdoplajdo
added
the 
bug
Something isn't working
label
Apr 25, 2026
andrej-jasso
andrej-jasso
reviewed
Jul 1, 2026
View reviewed changes
Comment threadREADME.md
@ViewModel var model: AnyReactor = MyViewModel().eraseToAnyReactor()
```

`AnyReactor` does not start the wrapped reactor automatically. Start it explicitly:
@andrej-jassoandrej-jassoJul 1, 2026

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

This needs to go to BREAKING CHANGES section after.
@andrej-jassoandrej-jassoJul 1, 2026

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

@plajdo can you please create an MD.file tracking the breaking changes for each version. Its easier to maintain if its part of the PR.
@plajdoplajdoJul 1, 2026

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

I cannot push to this branch anymore, I will create a new PR from fork. If changelog is needed, please create one separately.
However: I checked that this change should not affect any of company's projects, so no further changes are necessary (calling start manually was a recommended pattern anyway).
@andrej-jasso
andrej-jasso
requested a review
from CopilotJuly 1, 2026 11:10
CopilotAI
reviewed
Jul 1, 2026
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview
This PR aims to make Reactor.start() safe to call multiple times by avoiding duplicate external subscriptions, and changes AnyReactor lifecycle so the wrapped reactor is not auto-started (requiring explicit start() by callers).
Changes:
  • Added a subscription-existence guard to make Reactor.start() behave idempotently.
  • Disabled AnyReactor auto-start by removing base.start() from its initializer and updating its docs.
  • Updated README to instruct explicitly calling start() for AnyReactor.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
FileDescription
Sources/GoodReactor/Core/Reactor.swiftAdds guidance on transform() side effects and guards start() based on existing subscriptions.
Sources/GoodReactor/Core/Erased/AnyReactor.swiftRemoves implicit base.start() and updates lifecycle documentation for explicit starting.
README.mdDocuments explicit start() usage for AnyReactor.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment threadSources/GoodReactor/Core/Reactor.swift
Comment threadSources/GoodReactor/Core/Erased/AnyReactor.swift
Comment threadREADME.md
@plajdo
plajdo
commented
Jul 1, 2026•

edited

Loading

Copy link


Copy Markdown

ContributorAuthor
Cannot push to this branch anymore, let's follow the changes at #25.
@plajdoplajdo
closed this
Jul 1, 2026
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@andrej-jassoandrej-jassoandrej-jasso left review comments
Copilot code reviewCopilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@MatusMistrikMatusMistrikAwaiting requested review from MatusMistrik

Assignees

@plajdoplajdo

Labels

bugSomething isn't working

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

3 participants

@plajdo@andrej-jasso

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Highlight search terms from Google/DuckDuckGo/Bing referrer
(function() {
var ref = document.referrer;
var terms = [];
if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {
var url = new URL(ref);
var q = url.searchParams.get('q') || url.searchParams.get('p');
if (q) {
terms = q.split(/\s+/).filter(function(t) { return t.length > 2; });
}
}
if (terms.length === 0) return;
var style = document.createElement('style');
style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';
document.head.appendChild(style);
function highlight(node) {
if (node.nodeType === 3) { // text node
var text = node.textContent;
var found = false;
terms.forEach(function(term) {
var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi');
if (regex.test(text)) {
found = true;
var frag = document.createDocumentFragment();
var parts = text.split(regex);
parts.forEach(function(part, i) {
if (i % 2 === 0) {
frag.appendChild(document.createTextNode(part));
} else {
var span = document.createElement('span');
span.className = 'userscript-highlight';
span.textContent = part;
frag.appendChild(span);
}
});
node.parentNode.replaceChild(frag, node);
}
});
} else if (node.nodeType === 1 && node.childNodes) { // element
var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];
if (!skipTags.includes(node.tagName)) {
Array.from(node.childNodes).forEach(highlight);
}
}
}
highlight(document.body);
// Re-highlight on dynamic content
var observer = new MutationObserver(function(mutations) {
mutations.forEach(function(m) {
m.addedNodes.forEach(function(node) {
if (node.nodeType === 1 || node.nodeType === 3) highlight(node);
});
});
});
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
fix: Idempotent start by plajdo · Pull Request #23 · GoodRequest/GoodReactor · GitHub
Skip to content
fix: Idempotent start - #23
Closed
plajdo wants to merge 2 commits into
mainfrom 
fix/idempotent-start
Closed
fix: Idempotent start#23
plajdo wants to merge 2 commits into
mainfrom 
fix/idempotent-start
Conversation
@plajdo
@plajdoplajdo
commented
Apr 25, 2026•

edited

Loading

Copy link


Copy Markdown

Contributor
Added check whether any subscriptions for current Reactor are active. This makes start function idempotent for callers, and effectively prevents creating duplicate subscriptions.
Condition: transform function must be free of side effects, as empty transform will be called multiple times, until first subscription is created for the Reactor.
Disabled auto-start of AnyReactor type-erased type.

Previously this auto-start behaviour caused incorrectly stored Combine/non-combine subscriptions to external events from viewModels. Subscriptions were being stored under the wrapper type, which caused confusion and non-clear event routing.
Effects:
Call to viewModel.start() now has to be explicit and will always be forwarded to correct concrete Reactor type.
@plajdoplajdo
self-assigned this
Apr 25, 2026
@plajdoplajdo
added
the 
bug
Something isn't working
label
Apr 25, 2026
andrej-jasso
andrej-jasso
reviewed
Jul 1, 2026
View reviewed changes
Comment threadREADME.md
@ViewModel var model: AnyReactor = MyViewModel().eraseToAnyReactor()
```

`AnyReactor` does not start the wrapped reactor automatically. Start it explicitly:
@andrej-jassoandrej-jassoJul 1, 2026

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

This needs to go to BREAKING CHANGES section after.
@andrej-jassoandrej-jassoJul 1, 2026

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

@plajdo can you please create an MD.file tracking the breaking changes for each version. Its easier to maintain if its part of the PR.
@plajdoplajdoJul 1, 2026

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

I cannot push to this branch anymore, I will create a new PR from fork. If changelog is needed, please create one separately.
However: I checked that this change should not affect any of company's projects, so no further changes are necessary (calling start manually was a recommended pattern anyway).
@andrej-jasso
andrej-jasso
requested a review
from CopilotJuly 1, 2026 11:10
CopilotAI
reviewed
Jul 1, 2026
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview
This PR aims to make Reactor.start() safe to call multiple times by avoiding duplicate external subscriptions, and changes AnyReactor lifecycle so the wrapped reactor is not auto-started (requiring explicit start() by callers).
Changes:
  • Added a subscription-existence guard to make Reactor.start() behave idempotently.
  • Disabled AnyReactor auto-start by removing base.start() from its initializer and updating its docs.
  • Updated README to instruct explicitly calling start() for AnyReactor.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
FileDescription
Sources/GoodReactor/Core/Reactor.swiftAdds guidance on transform() side effects and guards start() based on existing subscriptions.
Sources/GoodReactor/Core/Erased/AnyReactor.swiftRemoves implicit base.start() and updates lifecycle documentation for explicit starting.
README.mdDocuments explicit start() usage for AnyReactor.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment threadSources/GoodReactor/Core/Reactor.swift
Comment threadSources/GoodReactor/Core/Erased/AnyReactor.swift
Comment threadREADME.md
@plajdo
plajdo
commented
Jul 1, 2026•

edited

Loading

Copy link


Copy Markdown

ContributorAuthor
Cannot push to this branch anymore, let's follow the changes at #25.
@plajdoplajdo
closed this
Jul 1, 2026
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@andrej-jassoandrej-jassoandrej-jasso left review comments
Copilot code reviewCopilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@MatusMistrikMatusMistrikAwaiting requested review from MatusMistrik

Assignees

@plajdoplajdo

Labels

bugSomething isn't working

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

3 participants

@plajdo@andrej-jasso

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Strip utm_, fbclid, gclid, etc. from all links on page
(function() {
var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',
'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',
'ref', 'ref_src', 'source', 'medium', 'campaign'];
function cleanUrl(url) {
try {
var u = new URL(url, window.location.origin);
var changed = false;
trackingParams.forEach(function(p) {
if (u.searchParams.has(p)) {
u.searchParams.delete(p);
changed = true;
}
});
return changed ? u.toString() : url;
} catch (e) {
return url;
}
}
function cleanLinks() {
document.querySelectorAll('a[href]').forEach(function(a) {
var clean = cleanUrl(a.href);
if (clean !== a.href) a.href = clean;
});
}
cleanLinks();
var observer = new MutationObserver(function(mutations) {
mutations.forEach(function(m) {
m.addedNodes.forEach(function(node) {
if (node.nodeType === 1) {
if (node.tagName === 'A') cleanLinks();
node.querySelectorAll('a[href]').forEach(function(a) {
var clean = cleanUrl(a.href);
if (clean !== a.href) a.href = clean;
});
}
});
});
});
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '
fix: Idempotent start by plajdo · Pull Request #23 · GoodRequest/GoodReactor · GitHub
Skip to content
fix: Idempotent start - #23
Closed
plajdo wants to merge 2 commits into
mainfrom 
fix/idempotent-start
Closed
fix: Idempotent start#23
plajdo wants to merge 2 commits into
mainfrom 
fix/idempotent-start
Conversation
@plajdo
@plajdoplajdo
commented
Apr 25, 2026•

edited

Loading

Copy link


Copy Markdown

Contributor
Added check whether any subscriptions for current Reactor are active. This makes start function idempotent for callers, and effectively prevents creating duplicate subscriptions.
Condition: transform function must be free of side effects, as empty transform will be called multiple times, until first subscription is created for the Reactor.
Disabled auto-start of AnyReactor type-erased type.

Previously this auto-start behaviour caused incorrectly stored Combine/non-combine subscriptions to external events from viewModels. Subscriptions were being stored under the wrapper type, which caused confusion and non-clear event routing.
Effects:
Call to viewModel.start() now has to be explicit and will always be forwarded to correct concrete Reactor type.
@plajdoplajdo
self-assigned this
Apr 25, 2026
@plajdoplajdo
added
the 
bug
Something isn't working
label
Apr 25, 2026
andrej-jasso
andrej-jasso
reviewed
Jul 1, 2026
View reviewed changes
Comment threadREADME.md
@ViewModel var model: AnyReactor = MyViewModel().eraseToAnyReactor()
```

`AnyReactor` does not start the wrapped reactor automatically. Start it explicitly:
@andrej-jassoandrej-jassoJul 1, 2026

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

This needs to go to BREAKING CHANGES section after.
@andrej-jassoandrej-jassoJul 1, 2026

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

@plajdo can you please create an MD.file tracking the breaking changes for each version. Its easier to maintain if its part of the PR.
@plajdoplajdoJul 1, 2026

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

I cannot push to this branch anymore, I will create a new PR from fork. If changelog is needed, please create one separately.
However: I checked that this change should not affect any of company's projects, so no further changes are necessary (calling start manually was a recommended pattern anyway).
@andrej-jasso
andrej-jasso
requested a review
from CopilotJuly 1, 2026 11:10
CopilotAI
reviewed
Jul 1, 2026
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview
This PR aims to make Reactor.start() safe to call multiple times by avoiding duplicate external subscriptions, and changes AnyReactor lifecycle so the wrapped reactor is not auto-started (requiring explicit start() by callers).
Changes:
  • Added a subscription-existence guard to make Reactor.start() behave idempotently.
  • Disabled AnyReactor auto-start by removing base.start() from its initializer and updating its docs.
  • Updated README to instruct explicitly calling start() for AnyReactor.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
FileDescription
Sources/GoodReactor/Core/Reactor.swiftAdds guidance on transform() side effects and guards start() based on existing subscriptions.
Sources/GoodReactor/Core/Erased/AnyReactor.swiftRemoves implicit base.start() and updates lifecycle documentation for explicit starting.
README.mdDocuments explicit start() usage for AnyReactor.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment threadSources/GoodReactor/Core/Reactor.swift
Comment threadSources/GoodReactor/Core/Erased/AnyReactor.swift
Comment threadREADME.md
@plajdo
plajdo
commented
Jul 1, 2026•

edited

Loading

Copy link


Copy Markdown

ContributorAuthor
Cannot push to this branch anymore, let's follow the changes at #25.
@plajdoplajdo
closed this
Jul 1, 2026
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@andrej-jassoandrej-jassoandrej-jasso left review comments
Copilot code reviewCopilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@MatusMistrikMatusMistrikAwaiting requested review from MatusMistrik

Assignees

@plajdoplajdo

Labels

bugSomething isn't working

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

3 participants

@plajdo@andrej-jasso

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Auto-enable theater mode on YouTube
(function() {
function tryTheater() {
var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]');
if (btn && !btn.classList.contains('activated')) {
btn.click();
}
}
// Try immediately
tryTheater();
// Try after navigation (SPA)
var lastUrl = location.href;
setInterval(function() {
if (location.href !== lastUrl) {
lastUrl = location.href;
setTimeout(tryTheater, 500);
}
}, 1000);
// Also try on player load
var observer = new MutationObserver(tryTheater);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
fix: Idempotent start by plajdo · Pull Request #23 · GoodRequest/GoodReactor · GitHub
Skip to content
fix: Idempotent start - #23
Closed
plajdo wants to merge 2 commits into
mainfrom 
fix/idempotent-start
Closed
fix: Idempotent start#23
plajdo wants to merge 2 commits into
mainfrom 
fix/idempotent-start
Conversation
@plajdo
@plajdoplajdo
commented
Apr 25, 2026•

edited

Loading

Copy link


Copy Markdown

Contributor
Added check whether any subscriptions for current Reactor are active. This makes start function idempotent for callers, and effectively prevents creating duplicate subscriptions.
Condition: transform function must be free of side effects, as empty transform will be called multiple times, until first subscription is created for the Reactor.
Disabled auto-start of AnyReactor type-erased type.

Previously this auto-start behaviour caused incorrectly stored Combine/non-combine subscriptions to external events from viewModels. Subscriptions were being stored under the wrapper type, which caused confusion and non-clear event routing.
Effects:
Call to viewModel.start() now has to be explicit and will always be forwarded to correct concrete Reactor type.
@plajdoplajdo
self-assigned this
Apr 25, 2026
@plajdoplajdo
added
the 
bug
Something isn't working
label
Apr 25, 2026
andrej-jasso
andrej-jasso
reviewed
Jul 1, 2026
View reviewed changes
Comment threadREADME.md
@ViewModel var model: AnyReactor = MyViewModel().eraseToAnyReactor()
```

`AnyReactor` does not start the wrapped reactor automatically. Start it explicitly:
@andrej-jassoandrej-jassoJul 1, 2026

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

This needs to go to BREAKING CHANGES section after.
@andrej-jassoandrej-jassoJul 1, 2026

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

@plajdo can you please create an MD.file tracking the breaking changes for each version. Its easier to maintain if its part of the PR.
@plajdoplajdoJul 1, 2026

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

I cannot push to this branch anymore, I will create a new PR from fork. If changelog is needed, please create one separately.
However: I checked that this change should not affect any of company's projects, so no further changes are necessary (calling start manually was a recommended pattern anyway).
@andrej-jasso
andrej-jasso
requested a review
from CopilotJuly 1, 2026 11:10
CopilotAI
reviewed
Jul 1, 2026
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview
This PR aims to make Reactor.start() safe to call multiple times by avoiding duplicate external subscriptions, and changes AnyReactor lifecycle so the wrapped reactor is not auto-started (requiring explicit start() by callers).
Changes:
  • Added a subscription-existence guard to make Reactor.start() behave idempotently.
  • Disabled AnyReactor auto-start by removing base.start() from its initializer and updating its docs.
  • Updated README to instruct explicitly calling start() for AnyReactor.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
FileDescription
Sources/GoodReactor/Core/Reactor.swiftAdds guidance on transform() side effects and guards start() based on existing subscriptions.
Sources/GoodReactor/Core/Erased/AnyReactor.swiftRemoves implicit base.start() and updates lifecycle documentation for explicit starting.
README.mdDocuments explicit start() usage for AnyReactor.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment threadSources/GoodReactor/Core/Reactor.swift
Comment threadSources/GoodReactor/Core/Erased/AnyReactor.swift
Comment threadREADME.md
@plajdo
plajdo
commented
Jul 1, 2026•

edited

Loading

Copy link


Copy Markdown

ContributorAuthor
Cannot push to this branch anymore, let's follow the changes at #25.
@plajdoplajdo
closed this
Jul 1, 2026
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@andrej-jassoandrej-jassoandrej-jasso left review comments
Copilot code reviewCopilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@MatusMistrikMatusMistrikAwaiting requested review from MatusMistrik

Assignees

@plajdoplajdo

Labels

bugSomething isn't working

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

3 participants

@plajdo@andrej-jasso

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Remove or un-stick sticky/fixed headers that block content
(function() {
function unstick() {
document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) {
if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {
el.style.position = 'static';
el.style.top = 'auto';
el.style.zIndex = 'auto';
}
});
}
unstick();
var observer = new MutationObserver(unstick);
observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });
})();
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
})();
fix: Idempotent start by plajdo · Pull Request #23 · GoodRequest/GoodReactor · GitHub
Skip to content
fix: Idempotent start - #23
Closed
plajdo wants to merge 2 commits into
mainfrom 
fix/idempotent-start
Closed
fix: Idempotent start#23
plajdo wants to merge 2 commits into
mainfrom 
fix/idempotent-start
Conversation
@plajdo
@plajdoplajdo
commented
Apr 25, 2026•

edited

Loading

Copy link


Copy Markdown

Contributor
Added check whether any subscriptions for current Reactor are active. This makes start function idempotent for callers, and effectively prevents creating duplicate subscriptions.
Condition: transform function must be free of side effects, as empty transform will be called multiple times, until first subscription is created for the Reactor.
Disabled auto-start of AnyReactor type-erased type.

Previously this auto-start behaviour caused incorrectly stored Combine/non-combine subscriptions to external events from viewModels. Subscriptions were being stored under the wrapper type, which caused confusion and non-clear event routing.
Effects:
Call to viewModel.start() now has to be explicit and will always be forwarded to correct concrete Reactor type.
@plajdoplajdo
self-assigned this
Apr 25, 2026
@plajdoplajdo
added
the 
bug
Something isn't working
label
Apr 25, 2026
andrej-jasso
andrej-jasso
reviewed
Jul 1, 2026
View reviewed changes
Comment threadREADME.md
@ViewModel var model: AnyReactor = MyViewModel().eraseToAnyReactor()
```

`AnyReactor` does not start the wrapped reactor automatically. Start it explicitly:
@andrej-jassoandrej-jassoJul 1, 2026

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

This needs to go to BREAKING CHANGES section after.
@andrej-jassoandrej-jassoJul 1, 2026

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

@plajdo can you please create an MD.file tracking the breaking changes for each version. Its easier to maintain if its part of the PR.
@plajdoplajdoJul 1, 2026

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

I cannot push to this branch anymore, I will create a new PR from fork. If changelog is needed, please create one separately.
However: I checked that this change should not affect any of company's projects, so no further changes are necessary (calling start manually was a recommended pattern anyway).
@andrej-jasso
andrej-jasso
requested a review
from CopilotJuly 1, 2026 11:10
CopilotAI
reviewed
Jul 1, 2026
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview
This PR aims to make Reactor.start() safe to call multiple times by avoiding duplicate external subscriptions, and changes AnyReactor lifecycle so the wrapped reactor is not auto-started (requiring explicit start() by callers).
Changes:
  • Added a subscription-existence guard to make Reactor.start() behave idempotently.
  • Disabled AnyReactor auto-start by removing base.start() from its initializer and updating its docs.
  • Updated README to instruct explicitly calling start() for AnyReactor.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
FileDescription
Sources/GoodReactor/Core/Reactor.swiftAdds guidance on transform() side effects and guards start() based on existing subscriptions.
Sources/GoodReactor/Core/Erased/AnyReactor.swiftRemoves implicit base.start() and updates lifecycle documentation for explicit starting.
README.mdDocuments explicit start() usage for AnyReactor.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment threadSources/GoodReactor/Core/Reactor.swift
Comment threadSources/GoodReactor/Core/Erased/AnyReactor.swift
Comment threadREADME.md
@plajdo
plajdo
commented
Jul 1, 2026•

edited

Loading

Copy link


Copy Markdown

ContributorAuthor
Cannot push to this branch anymore, let's follow the changes at #25.
@plajdoplajdo
closed this
Jul 1, 2026
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@andrej-jassoandrej-jassoandrej-jasso left review comments
Copilot code reviewCopilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@MatusMistrikMatusMistrikAwaiting requested review from MatusMistrik

Assignees

@plajdoplajdo

Labels

bugSomething isn't working

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

3 participants

@plajdo@andrej-jasso