Convert addon to use template tag #84

simonihmig · 2023-03-16T11:25:40Z

This converts all components of the addon itself (we only had this for the test-app before) to use template tags (gts).

changeset-bot · 2023-03-16T11:25:43Z

🦋 Changeset detected

Latest commit: 67a5169

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages

Name	Type
ember-headless-form	Patch
docs-app	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

github-actions · 2023-03-16T11:28:49Z

Preview URLs

Env: preview
Docs: https://c8db88c8.ember-headless-form.pages.dev

NullVoxPopuli · 2023-03-16T11:54:40Z

I just published 0.2.0 of the rollup plugin. I don't expect anything you've reported to be fixed, but i'll check this branch and see what's going on

ynotdraw · 2023-03-16T12:13:52Z

YES!!!!! 🎉

NullVoxPopuli · 2023-03-16T16:17:20Z

Glint / gts related type issues fixed here: #90
Looks like some real bugs are being revealed still!

and there are more work-arounds than I'm comfortable with in the types / imports -- I'll work towards fixing those

NullVoxPopuli · 2023-03-16T16:48:14Z

The other reported issue has a fix PR here: emberjs/babel-plugin-ember-template-compilation#16 (though, I don't have sufficient context and that could throw a wrench in things 😅 )

simonihmig · 2023-03-17T08:30:40Z

though, I don't have sufficient context and that could throw a wrench in things

Yeah, that's why I raised the issue first, without even trying to look into what needs to be fixed, as it seemed intentional for some reason. We'll see...

simonihmig · 2023-03-28T11:43:30Z

Linting is fixed. But Embroider is broken, due to embroider-build/embroider#1382

simonihmig · 2023-04-14T10:36:00Z

Linting is fixed. But Embroider is broken, due to embroider-build/embroider#1382

This is fixed now with embroider-build/embroider#1383. Therefore using a unstable release of Embroider now in our ember-try config, until we get the next stable release of Embroider!

ynotdraw

🚀 This is awesome!

ynotdraw · 2023-04-14T12:09:36Z

packages/ember-headless-form/src/-private/components/control/input.gts

I love seeing these components in a single file! This is great.

ynotdraw · 2023-04-14T12:11:18Z

packages/ember-headless-form/src/-private/utils.ts

+// this is copy pasted from https://github.com/emberjs/ember.js/blob/60d2e0cddb353aea0d6e36a72fda971010d92355/packages/%40ember/-internals/glimmer/lib/helpers/unique-id.ts
+// Unfortunately due to https://github.com/emberjs/ember.js/issues/20165 we cannot use the built-in version in template tags


Oh! This is good to know. Thanks for the helpful comment here

ynotdraw · 2023-04-14T12:19:22Z

packages/ember-headless-form/src/-private/components/field.gts

 }
+
+// eslint-disable-next-line @typescript-eslint/no-unused-vars -- workaround for unknown modifier helper: https://github.com/typed-ember/glint/issues/410
+declare const modifier: any;


Looks like a PR is up! typed-ember/glint#553 🎉

Yeah, I saw that also. And it was merged some hours ago! 🎉

ynotdraw · 2023-04-14T12:21:25Z

packages/ember-headless-form/unpublished-development-types/index.d.ts

-import type TemplateRegistry from '@glint/environment-ember-loose/registry';
-
-// importing this directly from the published types (https://github.com/embroider-build/embroider/blob/main/packages/util/index.d.ts) does not work,
-// see point 3 in Dan's comment here: https://github.com/typed-ember/glint/issues/518#issuecomment-1400306133
-declare class EnsureSafeComponentHelper<
-  // eslint-disable-next-line @typescript-eslint/no-explicit-any
-  C extends string | ComponentLike<any>
-> extends Helper<{
-  Args: {
-    Positional: [component: C];
-  };
-  Return: C extends keyof TemplateRegistry
-    ? TemplateRegistry[C]
-    : C extends string
-    ? ComponentLike<unknown>
-    : C;
-}> {}


All of this cleanup is excellent 🎉

simonihmig · 2023-04-17T17:21:55Z

Hey @camskene @joelamb @danwenzel, pulled you into this PR on a repo you might not have seen before, as Nicole (who together with Tony usually reviewed the stuff here) is out this week and I still need one more approval to be able to merge this! 😅. Would be nice if one of you could have a look, thanks!

danwenzel · 2023-04-18T17:07:44Z

packages/ember-headless-form/src/-private/components/control/checkbox.gts

+  <template>
+    <input
+      name={{@name}}
+      type="checkbox"


I'm surprised to see double quotes here, as we're now (sort of) in JS, and they were single quotes in the hbs... 🤷

Oh, didn't realise this before. Probably our prettier config is a bit inconsistent. I did wonder why we have single quotes in hbs before (which in other projects were usually double quotes)...

danwenzel · 2023-04-18T17:08:32Z

packages/ember-headless-form/src/-private/components/control/input.gts

@@ -1,5 +1,6 @@
 import Component from '@glimmer/component';
 import { assert } from '@ember/debug';
+import { on } from '@ember/modifier';


yay explicit imports! 🎉

danwenzel · 2023-04-18T17:09:57Z

packages/ember-headless-form/src/-private/components/control/radio-group.gts

+export interface HeadlessFormControlRadioGroupComponentSignature {
+  Element: HTMLDivElement;
+  Args: {
+    // the following are private arguments curried by the component helper, so users will never have to use those


ah, I like this comment and the @internal syntax. I ran into this with a component recently and was wondering if there was good messaging like this

danwenzel · 2023-04-18T17:13:53Z

packages/ember-headless-form/src/-private/components/control/select.gts

+      {{yield
+        (hash
+          Option=(component
+            HeadlessFormControlSelectOptionComponent selected=@value


don't need ensure-safe-component anymore?

Nope! 🎉

With strict mode templates, the {{component}} helper is also more strict, not allowing a string based lookup anymore. This makes sense as you would have explicit imports anyway. And by that, Embroider can safely assume that whatever is passed to {{component myComponent}} as the first argument is already something that has been imported somewhere (and as such it is already "seen" by Embroider/webpack and added to the bundle), so it does not need to care about this being potentially unsafe anymore. I also learned about this more recently...

simonihmig and others added 4 commits March 28, 2023 12:32

Convert addon to use template tag

bf614b1

Fix type violations due to missing glint configuration

8a03963

Build success

f7ae1ea

What a lint:fix adventure

2ae55e0

simonihmig force-pushed the gts-in-addon branch from dea25b0 to 18724cf Compare March 28, 2023 10:34

Fix types, cleanup

4ceb555

simonihmig force-pushed the gts-in-addon branch from 18724cf to 4ceb555 Compare March 28, 2023 10:47

simonihmig added 2 commits April 14, 2023 12:04

Convert all components to gts

e58f052

Run embroider scenarios using unstable

f1d8baa

simonihmig force-pushed the gts-in-addon branch from 016566b to f1d8baa Compare April 14, 2023 10:34

simonihmig marked this pull request as ready for review April 14, 2023 10:36

simonihmig requested review from nicolechung and ynotdraw April 14, 2023 10:36

Add changeset

67a5169

ynotdraw approved these changes Apr 14, 2023

View reviewed changes

simonihmig requested review from camskene, danwenzel and joelamb April 17, 2023 17:18

danwenzel approved these changes Apr 18, 2023

View reviewed changes

simonihmig merged commit dd97c0b into main Apr 18, 2023

simonihmig deleted the gts-in-addon branch April 18, 2023 17:28

github-actions bot mentioned this pull request Apr 18, 2023

Release Preview (beta) #81

Merged

simonihmig mentioned this pull request Jun 14, 2023

Add FileInput support to toucan-form CrowdStrike/ember-toucan-core#177

Merged

github-actions bot mentioned this pull request Dec 18, 2023

Release Preview #150

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Convert addon to use template tag #84

Convert addon to use template tag #84

simonihmig commented Mar 16, 2023 •

edited

Loading

changeset-bot bot commented Mar 16, 2023 •

edited

Loading

github-actions bot commented Mar 16, 2023 •

edited

Loading

NullVoxPopuli commented Mar 16, 2023

ynotdraw commented Mar 16, 2023

NullVoxPopuli commented Mar 16, 2023

NullVoxPopuli commented Mar 16, 2023

simonihmig commented Mar 17, 2023

simonihmig commented Mar 28, 2023

simonihmig commented Apr 14, 2023

ynotdraw left a comment

ynotdraw Apr 14, 2023

ynotdraw Apr 14, 2023

ynotdraw Apr 14, 2023

simonihmig Apr 14, 2023

ynotdraw Apr 14, 2023

simonihmig commented Apr 17, 2023

danwenzel Apr 18, 2023

simonihmig Apr 18, 2023

danwenzel Apr 18, 2023

danwenzel Apr 18, 2023

danwenzel Apr 18, 2023

simonihmig Apr 18, 2023

		// this is copy pasted from https://github.com/emberjs/ember.js/blob/60d2e0cddb353aea0d6e36a72fda971010d92355/packages/%40ember/-internals/glimmer/lib/helpers/unique-id.ts
		// Unfortunately due to https://github.com/emberjs/ember.js/issues/20165 we cannot use the built-in version in template tags

Convert addon to use template tag #84

Convert addon to use template tag #84

Conversation

simonihmig commented Mar 16, 2023 • edited Loading

changeset-bot bot commented Mar 16, 2023 • edited Loading

🦋 Changeset detected

github-actions bot commented Mar 16, 2023 • edited Loading

Preview URLs

NullVoxPopuli commented Mar 16, 2023

ynotdraw commented Mar 16, 2023

NullVoxPopuli commented Mar 16, 2023

NullVoxPopuli commented Mar 16, 2023

simonihmig commented Mar 17, 2023

simonihmig commented Mar 28, 2023

simonihmig commented Apr 14, 2023

ynotdraw left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

simonihmig commented Apr 17, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

simonihmig commented Mar 16, 2023 •

edited

Loading

changeset-bot bot commented Mar 16, 2023 •

edited

Loading

github-actions bot commented Mar 16, 2023 •

edited

Loading