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

export renderSettled via @ember/renderer package #20000

Merged

Conversation

cafreeman
Copy link
Contributor

Serves as the first step in the implementation of https://github.com/cafreeman/rfcs/blob/dont-use-this-for-template-values-in-tests/text/0785-remove-set-get-in-tests.md

This PR simply exposes an existing utility (which is already fully tested) in a public package so that it can be consumed in ember-test-helpers.

@jacobq
Copy link
Contributor

jacobq commented Mar 1, 2022

No fair! You got github item number 20000, you lucky 😆 (I got #20001 though)

@cafreeman
Copy link
Contributor Author

@jacobq my lucky day :)

Copy link
Contributor

@chriskrycho chriskrycho left a comment

Choose a reason for hiding this comment

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

In case anyone else is wondering, I had to refresh myself on the RFC. We are indeed introducing a new top-level module here:

In order to implement rerender, we would also expose the work done by @rwjblue on renderSettled as public API in a new @ember/renderer module so that it could then be imported and used by @ember/test-helpers without issuue [sic].

@chriskrycho chriskrycho merged commit 5ec7e24 into emberjs:master Mar 2, 2022
@chriskrycho
Copy link
Contributor

Thank you, @cafreeman!

@chriskrycho
Copy link
Contributor

chriskrycho commented Mar 11, 2022

Whoops! This should not have been merged yet, as we haven’t merged the RFC and it still needs the feature flag to go with it. We’ll get that later but sorted, probably early next week.

@kategengler
Copy link
Member

@chriskrycho Branch point should be next Monday (21st), so the flag is needed by then

@chriskrycho
Copy link
Contributor

cc @cafreeman

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants