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

Upgrade expect-type; fix issues it reveals; add RenderingTestContext #1286

Merged
merged 1 commit into from
Dec 15, 2022

Conversation

gitKrystan
Copy link
Collaborator

No description provided.

@param {Object} context the context to setup for rendering
@returns {Promise<Object>} resolves with the context that was setup
@param {TestContext} context the context to setup for rendering
@returns {Promise<RenderingTestContext>} resolves with the context that was setup
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

AFAICT no OSS code uses the value here. We could opt not to make it public.

Copy link
Contributor

Choose a reason for hiding this comment

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

Seems good to export, actually: it moves us one step closer to not having everything depend on this.<whatever>: even this.element becomes unnecessary if you do this:

import { module, test } from 'qunit';
import { setupRenderingTest } from 'ember-qunit'; // transitively is this
import { render } from '@ember/test-helpers';

module('cool', function (hooks) {
  let context = setupRenderingTest(hooks);

  test('neato', assert function (assert) {
    await render(<template>Hello!</template>);
    assert(context.element.innerText.includes("Hello!"));
  });
});

@param {Object} context the context to setup for rendering
@returns {Promise<Object>} resolves with the context that was setup
@param {TestContext} context the context to setup for rendering
@returns {Promise<RenderingTestContext>} resolves with the context that was setup
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems good to export, actually: it moves us one step closer to not having everything depend on this.<whatever>: even this.element becomes unnecessary if you do this:

import { module, test } from 'qunit';
import { setupRenderingTest } from 'ember-qunit'; // transitively is this
import { render } from '@ember/test-helpers';

module('cool', function (hooks) {
  let context = setupRenderingTest(hooks);

  test('neato', assert function (assert) {
    await render(<template>Hello!</template>);
    assert(context.element.innerText.includes("Hello!"));
  });
});

@chriskrycho
Copy link
Contributor

This one will also need to have a merge or rebase!

@chriskrycho chriskrycho changed the title Upgrade expect-type and fix issues the new version reveals Upgrade expect-type; fix issues it reveals; add RenderingTestContext Dec 15, 2022
@chriskrycho chriskrycho merged commit b900789 into emberjs:master Dec 15, 2022
@gitKrystan gitKrystan deleted the upgrade-expect-type branch December 15, 2022 22:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants