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

improve component blueprint for octane #218

Open
wants to merge 34 commits into
base: master
Choose a base branch
from

Conversation

BryanCrotaz
Copy link

@BryanCrotaz BryanCrotaz commented Nov 15, 2020

Cleaned up from #213

(cherry picked from commit 9d721f8)
remove comment

Co-authored-by: Chris Krycho <hello@chriskrycho.com>
(cherry picked from commit 3e86833)
(cherry picked from commit d152c31)
(cherry picked from commit ef7d292)
(cherry picked from commit 9f0269e)
(cherry picked from commit 3c59cf8)
(cherry picked from commit 76c8799)
(cherry picked from commit 58d6499)
(cherry picked from commit dc4c154)
@BryanCrotaz BryanCrotaz changed the title Octane 3 improve component blueprint for octane Nov 15, 2020
@BryanCrotaz
Copy link
Author

@chriskrycho let me know when to rebase

@chriskrycho
Copy link
Member

Thanks, @BryanCrotaz – will ping you as soon as the other maintainers have had a chance to look at #216 and and #217 and therefore we merge them! (I try to practice good PR hygiene on things I maintain: stuff doesn't go in without a review. 😅)

@chriskrycho
Copy link
Member

@BryanCrotaz all right, this is now in need of a merge or rebase—I unfortunately had to touch some of the test files to get things all working again. Once your rebase, though, you should be able to drop a fair bit from this branch. Sorry about the hassle, and thanks again for the contribution! Ping me once you have it working, and we'll get it landed!

@chriskrycho
Copy link
Member

Also, fair warning: I'm working on actually getting all the Renovate-bot-powered dependency updates landed today, so you may just want to wait till those land, or you'll have an obnoxious time with rebase/merge issues from package.json and yarn.lock getting out of date.

@BryanCrotaz
Copy link
Author

#216 deleted all the component blueprints - was that deliberate?

@MrChocolatine
Copy link

MrChocolatine commented Nov 16, 2020

@BryanCrotaz , it was apparently:
8c6a016 (#216)

@chriskrycho
Copy link
Member

Yep, intentional precisely so that I wouldn't just end up duplicating your work. Re-adding them here is what we need!

@chriskrycho
Copy link
Member

…still hammering away at merging those other ones; once all the PRs marked chore(deps) or fix(deps) are closed, then you'll want to merge once more time and we can land this (assuming it's ✅).

@chriskrycho
Copy link
Member

And we are all caught up! Rebase and let's do this thing!

@BryanCrotaz
Copy link
Author

@chriskrycho can you see why there's a parsing error in the component test template? I've isolated it to the <% if ... %> <% else if ... %> construct but can't see the typo.

SyntaxError: Unexpected token ) (Error in blueprint template: /home/runner/work/ember-cli-typescript-blueprints/ember-cli-typescript-blueprints/blueprints/component-test/qunit-files/__root__/__testType__/__path__/__test__.ts)

@chriskrycho
Copy link
Member

Oh man! I have been there. Believe it or not, running in Docker may be substantially faster (for Ember stuff in general). It ends up absorbing a lot of the file system operations in RAM before flushing them to disk (only way it can do stuff in a reasonably speedy way), and… the virtual file system ends up being substantially faster than the Windows native file system. Which, you know, is bonkers. But it’s also true.

One of us will take a peek at this in the next couple days. I have a confirmed COVID exposure so not sure exactly what my own next few days will look like. 😬

@@ -0,0 +1,34 @@
import { expect } from 'chai';
Copy link
Member

Choose a reason for hiding this comment

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

This is still wrong—until you replace it with the correct file, stuff is going to be broken. Also, while you're at it, go ahead and remove all the shenanigans around supporting unit tests. As noted on the QUnit test blueprint, we only care here about integration tests, esp. b/c unit tests straight-up don't work with Glimmer components (as it should be!).

@BryanCrotaz
Copy link
Author

To clarify, @chriskrycho am I dropping support for anything except glimmer?

@chriskrycho
Copy link
Member

No, we should support generating Ember components the same as the core blueprints do. We just don’t want to generate unit tests for them.

@BryanCrotaz
Copy link
Author

My brain is melting. So much undocumented stuff in Ember blueprints.

Any idea what the difference between files and native_files is?

And any idea why the test for glimmer being present is succeeding in all tests, even the ember classic components ones?

@BryanCrotaz
Copy link
Author

Fancy pairing on this @chriskrycho ?

@chriskrycho
Copy link
Member

I’m off this week, and spending what OSS time I have chosen to use on getting our docs finally up to date – see typed-ember/ember-cli-typescript#935 – but perhaps next week.

@MrChocolatine
Copy link

Anything new here?

@chriskrycho
Copy link
Member

I may be able to help a little in a week or two. And perhaps @jamescdavis might be able to? If not, folks will just have to keep plugging away independently in the meantime.

@knownasilya
Copy link

🦜

@lifeart
Copy link

lifeart commented Jan 15, 2021

image

@atsjj
Copy link

atsjj commented Feb 3, 2021

I'm trying to get up-to-speed on what's going on with this PR and seeing if there's anything that the community can do to hop in on and get this stable for merge?

Looks like a rebase is needed as well as just getting tests passing? Not sure if that's all?

@lifeart
Copy link

lifeart commented Feb 25, 2021

@BryanCrotaz we need you! If I can help somehow, just let me know.

@BryanCrotaz
Copy link
Author

BryanCrotaz commented Feb 25, 2021

I'm out of my depth on the blueprints themselves. I think this needs a pair or group effort. Happy to jump on a zoom with someone and get this all cleaned up. I'm flat out until tomorrow night but could find some time over the weekend or Monday.

Current ember app for this week: image

@Alonski
Copy link

Alonski commented Mar 17, 2021

So... Any ideas if we will ever merge this on or are we waiting for the Typescriptification of Ember for this? 😋

@SergeAstapov
Copy link
Contributor

Considering number of changed files in PR, it's age and number of feedback/concerns, does it make sense to split into pieces?
e.g. work with one entity at a time so we could land things sooner.

Another thought, we may at least update existing files like https://github.com/typed-ember/ember-cli-typescript-blueprints/blob/master/blueprints/model/files/__root__/__path__/__name__.ts in parallel and separately from the (definitely great!) work done here.

@BryanCrotaz
Copy link
Author

Happy to help out but I reached beyond the end of my blueprint understanding, so I'd just be happy to see my partial work get finished by someone keen

@BryanCrotaz
Copy link
Author

Considering number of changed files in PR, it's age and number of feedback/concerns, does it make sense to split into pieces? e.g. work with one entity at a time so we could land things sooner.

Another thought, we may at least update existing files like https://github.com/typed-ember/ember-cli-typescript-blueprints/blob/master/blueprints/model/files/__root__/__path__/__name__.ts in parallel and separately from the (definitely great!) work done here.

Did any of this happen?

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.

8 participants