Skip to content
This repository was archived by the owner on Dec 31, 2022. It is now read-only.

improve component blueprint for octane #213

Closed
wants to merge 14 commits into from

Conversation

BryanCrotaz
Copy link

Bring up to date with ember blueprints and strongly type this in tests
Bring templates into components folder too

Copy link
Member

@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.

Thank you! Many, many times over, thank you! I’m very happy to land this as it is, and we can keep iterating.

// add your test properties here
}

moduleForComponent('<%= componentPathName %>', '<%= friendlyTestDescription %>', {
Copy link
Member

Choose a reason for hiding this comment

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

One thought: while we’re at it, we should, uhh, just get rid of the old-style moduleFor* blueprints. YIKES. I’d forgotten just how out of date our blueprints are. (Doesn’t have to be this PR.)

// anything which *must* be merged to prototype here
}) {
// normal class body definition here
export default class <%= classifiedModuleName %> extends Component {
Copy link
Member

Choose a reason for hiding this comment

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

Let’s delete module unification files as well (again, doesn’t have to be this PR). Module unification is dead, long live template imports. 😅

Copy link
Author

Choose a reason for hiding this comment

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

Make sure everyone who's using module unification has stopped using it before doing that...

Copy link
Author

Choose a reason for hiding this comment

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

Or throw an error with a link to a codemod that migrates the old fashioned folk up to an octane layout

Copy link
Member

Choose a reason for hiding this comment

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

While that’d be a nice thing to do I don’t think we need to do it. When we do it, we will cut a major release and note that we’ve dropped support for MU. Ember itself stopped supporting it in the blueprints long since, I believe, and we don’t want to support generating more code that way from our POV. We already dropped support for it in a variety of places in the core addon.

@chriskrycho
Copy link
Member

@BryanCrotaz I will merge this "as is" as soon as you fix the linting issues here!

@chriskrycho
Copy link
Member

Note, you may also end up wanting to rebase as I land a handful of updates to the repo today!

chriskrycho added a commit that referenced this pull request Nov 14, 2020
BREAKING CHANGE: drops all support for pre-[RFC 0232][rfc] blueprints,
since these have long since been replaced.

Note that this commit also removes the component blueprints entirely, in
favor of landing updated versions of them in [another PR][pr].

[rfc]: https://emberjs.github.io/rfcs/0232-simplify-qunit-testing-api.html
[pr]: #213
chriskrycho added a commit that referenced this pull request Nov 14, 2020
BREAKING CHANGE: drops all support for pre-[RFC 0232][rfc] blueprints,
since these have long since been replaced.

Note that this commit also removes the component blueprints entirely, in
favor of landing updated versions of them in [another PR][pr].

[rfc]: https://emberjs.github.io/rfcs/0232-simplify-qunit-testing-api.html
[pr]: #213
@BryanCrotaz
Copy link
Author

It appears that no commits were made to master, so no rebase needed?

@chriskrycho
Copy link
Member

The rebase will be needed, but isn’t yet. We will land #216 and #217 first, since those should make it easier to land this one CI-wise!

@BryanCrotaz
Copy link
Author

closed in favour of #218

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

Successfully merging this pull request may close these issues.

3 participants