Update documentation for native imports & signatures by dfreeman · Pull Request #307 · typed-ember/glint

dfreeman · 2022-04-21T12:14:16Z

Currently all of our documentation is framed in terms of the custom re-exports we've had to use for much for Glint's lifetime, but with #283 native imports are now an option, and with #304 those imports will be the only option. Accordingly, this PR updates the documentation site to use native imports and signatures in all of its example snippets.

This also removes the bulk of the contents of our README (from which the doc site contents were originally copied) in favor of nudging folks toward the more structured version of the content and avoiding the need to maintain the same information two places.

jamescdavis

We decided to teach type aliases for args in class-based helper and modifier signatures.

jamescdavis · 2022-04-21T15:47:02Z

+  Element: HTMLCanvasElement;
+  Args: {
+    Positional: Positional;
+    Named: RenderOptions;


jamescdavis · 2022-04-21T15:57:37Z

+type Positional = Array<number>;
+type Named = { andThenMultiplyBy?: number };
+
+export interface AddSignature {


Since signatures don't actually need to be exported, is there any value in doing so?

People sometimes like to do type gymnastics with them, and reverse engineering them from the component type itself generally makes TS fall over. Given the number of times I've had to point people to just exporting the signature type instead of trying to extract it, it seemed reasonable to do it by default.

gitKrystan

My review includes comments on this PR as well as comments on the state of the Glint+Ember docs in general (for you to take or leave).

gitKrystan · 2022-04-21T18:05:42Z

@@ -7,8 +7,8 @@

 - Ember


GitHub won't allow me to make comments on parts of the code that didn't change, so I'll keep track of those here:

glint/docs/getting-started.md

Lines 3 to 4 in 4fc9b4b

You'll first need to add `@glint/core` and an appropriate Glint environment to your project's `devDependencies`.

Check the section in the sidebar to the left for details about the specifics of working with Glint in a GlimmerX or Ember.js project.

This section is a bit confusing to me because the generic examples aren't super useful. Instead, I would emphasize links to the ember/glimmerx-specific installation guides here and removing the generic .glintrc.yml example. I remember the first time I read this documentation, I missed the sentence about looking for specific example in the sidebar and was confused by a-glint-environment in the example.

glint/docs/getting-started.md

Line 22 in 4fc9b4b

The `@glint/core` package includes two executables: `glint` and `glint-language-server`.

In order to make "getting started" seem super simple for the typical user, I wonder if it would be useful to remove mention of the language server there, since most users won't be working with it directly. I recommend moving the GLS references here to a separate top-level heading for language-server-devs.

glint/docs/ember/installation.md

Line 1 in 4fc9b4b

To use Glint with [Ember](https://github.com/emberjs/ember.js), you'll add the `@glint/core` and `@glint/environment-ember-loose` packages to your project's `devDependencies`, then create a `.glintrc.yml` in the root of your project.

Since Glint currently only works with TypeScript, it might be useful to link to ember-cli-typescript here, lest people try to roll their own Ember TypeScript config.

glint/docs/ember/installation.md

Line 30 in 4fc9b4b

Note that specifying `include` globs is optional, but may be a useful way to incrementally migrate your project to Glint over time.

The include config was super useful for my "Converting your Ember App to TS" tutorial, but I found it difficult to find the right combination of include globs (e.g. do you only need to include hbs files or do you have to include all of your TS as well?) and other temporary type changes (a la https://docs.ember-cli-typescript.com/ts/using-ts-effectively#opt-in-unsafety) to make glint pass while in the middle of my migration. It might be useful to include a top-level section with more details here.

glint/docs/ember/component-signatures.md

Line 32 in 4fc9b4b

export default class SuperTable<T> extends Component<SuperTableSignature<T>> {}

NIT: I recommend adding /* ... */ to indicate that code is redacted.

export default class SuperTable<T> extends Component<SuperTableSignature<T>> {/*...*/}`

Specifically, in this location, it makes this component look like a template-only component, so the documentation appears to contradict the template-only documentation below. (Reading the code, though, it looks like it would work w/ the "empty" class as shown, but I'm not sure if that's the intent of the documentation since there's a separate section for template-only components.)

glint/docs/ember/template-registry.md

Line 1 in 4fc9b4b

Because Ember's template resolution occurs dynamically at runtime today, Glint needs a way of mapping the names used in your templates to the actual backing value they'll be resolved to. This takes the form of a "type registry" similar to the one that powers Ember Data's types.

Consider linking to https://docs.ember-cli-typescript.com/ember-data for context.

Since Glint currently only works with TypeScript

Glint actually works with JS (#199), it's just not documented yet (#208).

Glint actually works with JS (#199), it's just not documented yet (#208).

Awesome! I do still think a link to e-cli-ts would be useful somewhere since people frequently use them together. My concern is that someone might assume they only need Glint since the overview says "Glint's CLI provides a template-aware tool for performing end-to-end TypeScript typechecking"

gitKrystan · 2022-04-21T18:34:50Z

+compatible with the DOM element(s) they're ultimately attached to. If no `Element` is specified, it will be a type error
+to set any HTML attributes or modifiers when invoking your component.
+
+The `Blocks` field specifies the names of any blocks the component yields to, as well as the type of any parameter(s)


Need to document how to specify the default block.

gitKrystan · 2022-04-21T18:38:08Z

+  // and a `row` block which will be invoked with each item and
+  // its index sequentially.
+  Blocks: {
+    header: [];


The template for this example indicates that the header is optional, but you've removed the ? here. If the ? is included, does Glint actually know about the optional-ness of the block? If so, seems useful to mention here.

This was a bit of intentional cleanup from a much earlier version of Glint's internals. Today Glint doesn't have a notion of required-ness for blocks, since {{yield}}-ing to a block that your consumer didn't actually pass is just a noop. #149 has a bit more detail on that

gitKrystan · 2022-04-21T18:50:58Z

@@ -0,0 +1,151 @@
+Like components, helpers and modifiers can also have their behavior in templates described by a `Signature` type.


Very grateful for the addition of this section.

gitKrystan · 2022-04-21T18:55:06Z


 The recommended approach is to include a declaration in each component, modifier or helper module that adds it to the registry, which is the default export of `@glint/environment-ember-loose/registry`.

+With [first-class component templates][fccts], the day is coming when we won't need this anymore, because any components/helpers/modifiers you use will already be statically in scope, but for now this approach ensures compatibility with the effective global scope of loose-mode templates.


This paragraph could use more visual emphasis.

gitKrystan · 2022-04-21T19:01:05Z

@@ -2,12 +2,16 @@ Because Ember's template resolution occurs dynamically at runtime today, Glint n

 The recommended approach is to include a declaration in each component, modifier or helper module that adds it to the registry, which is the default export of `@glint/environment-ember-loose/registry`.


This section could use an example of what the glint error message will look like if you forget to add something to the registry so that Google will point someone to this documentation when that happens.

gitKrystan · 2022-04-21T19:10:08Z

@@ -5,19 +5,34 @@ Here's an example that provides types for the `page-title` helper and the `Welco
 {% code title="types/global.d.ts" %}


This section could use another reference to https://github.com/Gavant/glint-template-types.

chriskrycho · 2022-04-22T00:53:24Z

-```
-
-You can also use the `glint` command locally with the `--watch` flag to monitor your project as you work!
+You can [use the `glint` executable in CI][using-glint] to ensure you maintain type safety in your templates. You can also use the `glint` command locally with the `--watch` flag to monitor your project as you work!


The wording here is 🤔 – "maintain type safety in your templates" sort of reads to me as if it is only for templates. Trying to think of a rephrasing and so far failing, though.

Co-authored-by: Krystan HuffMenne <krystan@tilde.io>

dfreeman · 2022-04-22T12:58:30Z

Thanks for the feedback, everyone! I REALLY appreciate the attention on the docs ❤️

In the interest of keeping the scope of this PR from blowing out (despite what the diff looks like, it really is 95% find/replacing code snippets and moving existing prose around between .md files), I'm going to merge this as it stands to accomplish the original goal of unblocking our 0.8.0 release.

I would love to review PRs incorporating the other suggestions here (as well as anything else that can be improved!), but I know you're all busy and have already devoted time to writing up your commentary here, so I'll plan to come back and revisit after I travel next week if better writers than me don't get to it first 🙂

feedback incorporated

Co-authored-by: Krystan HuffMenne <krystan@tilde.io>

I believe as of typed-ember#307 we don't need this anymore!?

dfreeman added 2 commits April 21, 2022 13:50

Remove section on environment-ember-loose custom imports

3ef5605

Migrate Ember docs to new imports + signature formats

f21af18

dfreeman added the documentation Improvements or additions to documentation label Apr 21, 2022

dfreeman mentioned this pull request Apr 21, 2022

Update README, renaming Yields -> Blocks #295

Closed

jamescdavis previously requested changes Apr 21, 2022

View reviewed changes

Comment thread docs/ember/helper-and-modifier-signatures.md Outdated

Comment thread docs/ember/helper-and-modifier-signatures.md Outdated

dfreeman added 5 commits April 21, 2022 16:39

Add missing helper/modifier examples to the docs

af12ee7

Pare down README contents and guide readers to the docs site

084caee

Add a note that GlimmerX won't be compatible with 0.8 yet

f05941a

Add 0.7 -> 0.8 migration notes

1d6acbe

Call out environment import recommendations more prominently

02f98ce

dfreeman force-pushed the native-import-docs branch from 7093d99 to 02f98ce Compare April 21, 2022 14:40

dfreeman requested a review from jamescdavis April 21, 2022 15:04

dfreeman mentioned this pull request Apr 21, 2022

Mark environment-glimmerx temporarily private to avoid publishing #308

Merged

jamescdavis reviewed Apr 21, 2022

View reviewed changes

gitKrystan reviewed Apr 21, 2022

View reviewed changes

chriskrycho reviewed Apr 22, 2022

View reviewed changes

Comment thread docs/ember/component-signatures.md

Fix typo

58c09ca

Co-authored-by: Krystan HuffMenne <krystan@tilde.io>

Fix README typo

59b36bf

Co-authored-by: Krystan HuffMenne <krystan@tilde.io>

dfreeman merged commit 81745f4 into main Apr 22, 2022

dfreeman deleted the native-import-docs branch April 22, 2022 13:18

simonihmig added a commit to simonihmig/glint that referenced this pull request Apr 28, 2022

Remove re-exports from known limitations

f6cb22d

I believe as of typed-ember#307 we don't need this anymore!?

simonihmig mentioned this pull request Apr 28, 2022

Remove re-exports from known limitations #312

Merged

gitKrystan mentioned this pull request May 13, 2022

DOCS: Clarify Getting Started and Ember Installation pages #326

Merged

	You'll first need to add `@glint/core` and an appropriate Glint environment to your project's `devDependencies`.
	Check the section in the sidebar to the left for details about the specifics of working with Glint in a GlimmerX or Ember.js project.

		@@ -0,0 +1,151 @@
		Like components, helpers and modifiers can also have their behavior in templates described by a `Signature` type.


		The recommended approach is to include a declaration in each component, modifier or helper module that adds it to the registry, which is the default export of `@glint/environment-ember-loose/registry`.

		With [first-class component templates][fccts], the day is coming when we won't need this anymore, because any components/helpers/modifiers you use will already be statically in scope, but for now this approach ensures compatibility with the effective global scope of loose-mode templates.

		@@ -2,12 +2,16 @@ Because Ember's template resolution occurs dynamically at runtime today, Glint n

		The recommended approach is to include a declaration in each component, modifier or helper module that adds it to the registry, which is the default export of `@glint/environment-ember-loose/registry`.

		@@ -5,19 +5,34 @@ Here's an example that provides types for the `page-title` helper and the `Welco
		{% code title="types/global.d.ts" %}

Conversation

dfreeman commented Apr 21, 2022

Uh oh!

jamescdavis left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gitKrystan left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dfreeman commented Apr 22, 2022

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

gitKrystan left a comment •

edited

Loading