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

fix!: rendered meaning #7747

Merged
merged 4 commits into from
Jan 10, 2024

Conversation

BeksOmega
Copy link
Collaborator

@BeksOmega BeksOmega commented Jan 4, 2024

The basics

The details

Resolves

Fixes #4288

Proposed Changes

Makes it so that rendered is constant, and is only used to check whether the block is a Block or a BlockSvg.

Reason for Changes

Makes the system easier to reason about and understand.

Test Coverage

Fixed change detectors.

Documentation

This will need docs. I have separate tasks filed for that.

Additional Information

This PR creates a pretty significant performance degredation related to bumpNeighbours I have a separate fix in the works for that. But in an effort to keep my PRs small I'm putting that up separately.

There's also more work to be removing references to initSvg and rendered and cleaning up initSvg.

Dependent on #7745

Breaking changes / To fix

If you are using rendered to check whether the block is initialized or currently visible, do not do that. rendered should only be used to tell if the block is a Block instance or a BlockSvg instance.

If you are setting rendered to temporarily disable rendering and improve performance, that is no longer necessary because we perform rendering batching.

If you have another use case for accessing rendered that is not covered here, please let us know on the forum.

@BeksOmega BeksOmega requested a review from a team as a code owner January 4, 2024 21:46
@BeksOmega BeksOmega requested a review from maribethb January 4, 2024 21:46
@github-actions github-actions bot added breaking change Used to mark a PR or issue that changes our public APIs. PR: fix Fixes a bug labels Jan 4, 2024
@BeksOmega BeksOmega marked this pull request as draft January 4, 2024 21:46
@github-actions github-actions bot added breaking change Used to mark a PR or issue that changes our public APIs. PR: fix Fixes a bug and removed PR: fix Fixes a bug breaking change Used to mark a PR or issue that changes our public APIs. labels Jan 4, 2024
@BeksOmega BeksOmega changed the base branch from rc/v11.0.0 to v11-breaks January 8, 2024 22:24
@github-actions github-actions bot added breaking change Used to mark a PR or issue that changes our public APIs. PR: fix Fixes a bug and removed PR: fix Fixes a bug breaking change Used to mark a PR or issue that changes our public APIs. labels Jan 8, 2024
@BeksOmega BeksOmega force-pushed the fix/block-initialization-2 branch from 3998b14 to e0f9525 Compare January 8, 2024 22:26
@github-actions github-actions bot added breaking change Used to mark a PR or issue that changes our public APIs. PR: fix Fixes a bug and removed PR: fix Fixes a bug breaking change Used to mark a PR or issue that changes our public APIs. labels Jan 9, 2024
@BeksOmega BeksOmega mentioned this pull request Jan 9, 2024
1 task
@github-actions github-actions bot added breaking change Used to mark a PR or issue that changes our public APIs. PR: fix Fixes a bug and removed PR: fix Fixes a bug breaking change Used to mark a PR or issue that changes our public APIs. labels Jan 9, 2024
@BeksOmega BeksOmega force-pushed the fix/block-initialization-2 branch from e0f9525 to 902aeae Compare January 9, 2024 22:01
@BeksOmega BeksOmega changed the title fix!: block initialization fix!: rendered meaning Jan 9, 2024
@github-actions github-actions bot added breaking change Used to mark a PR or issue that changes our public APIs. PR: fix Fixes a bug and removed PR: fix Fixes a bug breaking change Used to mark a PR or issue that changes our public APIs. labels Jan 9, 2024
@BeksOmega BeksOmega marked this pull request as ready for review January 9, 2024 22:05
@@ -123,7 +123,6 @@ export class Input {

if (this.sourceBlock.rendered) {
Copy link
Contributor

Choose a reason for hiding this comment

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

would it be possible to just check instanceof BlockSvg then you wouldn't need to cast either? circular dependencies are the only reason i can think of you wouldn't do that but i'm hoping that doesn't apply here

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It causes circular dependencies.

Copy link
Contributor

Choose a reason for hiding this comment

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

:(

@BeksOmega BeksOmega merged commit 3e3eb28 into google:v11-breaks Jan 10, 2024
6 checks passed
BeksOmega added a commit that referenced this pull request Jan 23, 2024
* fix: make rendered strictly for differentiating blocksvgs

* chore: fix references to rendered

* chore: fix tests

* chore: delete TODO
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change Used to mark a PR or issue that changes our public APIs. PR: fix Fixes a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants