Generated type declarations #189

theengineear · 2024-10-09T22:32:51Z

Generate “x-element.d.ts” from JSDoc comments.

Previously, the “x-element.d.ts” file was hand-curated, which was prone
to falling out of date.

Because the TypeScript team manages JSDoc, it’s fairly straightforward
to properly “type” your JS with JSDoc and output declarations which can
be used by folks writing TypeScript — all of that without needing to
actually author TypeScript ourselves.†

Additionally, tools like JSR will build basic documentation for
libraries which author JSDoc comments, so some wins there.

Finally, to ensure best practices for our JSDocs, we enable some
recommended rules from the eslint plugin.

† One goal of “x-element” is to be highly portable, it’s why no
dependencies exist and why the “x-element.js” file can be used
verbatim without a build step.

theengineear · 2024-10-09T22:34:44Z

x-element.d.ts

@@ -1,69 +1,239 @@
-export class XElement extends HTMLElement {


Here’s a good example — this should have said export default class XElement extends HTMLElement { (see the default). Easy to miss since we often don’t depend on this module. Generating it from well-considered JSDoc “types” feels like a better way to ensure this stays accurate.

theengineear · 2024-10-09T22:37:03Z

x-element.d.ts

+    static template(html: Function, engine: {
+        [key: string]: Function;
+    }): (properties: object, host: HTMLElement) => any;
+    static "__#1@#analyzeConstructor"(constructor: any): void;


… This, is annoying. TypeScript does want to track private fields for [reasons], so as far as I can tell, you cannot remove this stuff.

theengineear · 2024-10-09T22:38:00Z

package.json

@@ -3,11 +3,12 @@
  "description": "A dead simple starting point for custom elements.",
  "version": "1.0.0",
  "license": "Apache-2.0",
-  "repository": "https://github.com/Netflix/x-element",
+  "repository": "github:Netflix/x-element",


I went through some of the npm documentation again and tried to further “modernize” this package.json — worth further scrutiny for sure though.

theengineear · 2024-10-09T22:39:35Z

package.json

@@ -17,19 +18,23 @@
    "lint": "eslint --max-warnings=0 .",
    "lint-fix": "eslint --fix .",
    "test": "node test.js | tap-parser -l",
+    "build": "tsc",


Still need to figure out how we want to include this output…

@theengineear ~~Do you need to? If you're using JSDoc, TypeScript will import the file as typed.~~

Nevermind, I thought that worked but I can't get it to work consistently locally. Any reason it couldn't be stored right next to the output file in the NPM package/published directory, and stored that way? That's how most libs do it, as far as I've seen.

Since the entire interface is really a single class — I do think it makes sense to keep this checked into source as a sibling of the file itself. It allows us to scrutinize any changes that could happen there.

That said… that’s just my current opinion there. It’s completely subject to change if it ends up being overly cumbersome!

theengineear · 2024-10-09T22:40:37Z

tsconfig.tsbuildinfo

@@ -0,0 +1 @@
+{"root":["./x-element.js"],"version":"5.6.3"}


Not sure what this is all about. Can we .gitignore this?

@theengineear You could merge the contents into the tsconfig.json. This file is helpful for splitting the project into multiple project references. However, TypeScript also just allows you to define these references in the tsconfig.json if you need them anyway. It just supports allowing you using a separate config file, for which this file name is the default. I don't know why someone would choose to maintain a separate file from ts-config-- that probably makes more sense from the perspective of very large projects.

Anyway, since x-element is intended to be a single file import, I don't imagine you'll need this unless you're planning to add accessory projects to the repo. I'd just delete it for now. The version property doesn't appear to be an officially supported property for tsconfig, and you're already using the files attribute instead of root.

Thanks for the detailed response! I expected to be the one answering my own question there 😅

~~I’ll ignore the file 👌~~ Scratch that — Looks like I created it earlier when I must have been playing around with some configuration. Just deleted it 🤘

theengineear · 2024-10-09T22:41:39Z

Another step I’d like to take is publishing this branch to JSR as throw-away exercise. I’d be curious to see if it significantly improves the experience there.

Kelketek · 2024-10-11T01:22:04Z

x-element.js

@@ -899,110 +975,122 @@ export default class XElement extends HTMLElement {
  static #prototypeInterface = new Set(Object.getOwnPropertyNames(XElement.prototype));
 }

-/** Wrapper to document public interface to the default templating engine. */
-class TemplateEngine {
+// TODO: If / when we remove `repeat` in favor of `map` — we can probably delete


What needs to happen in order for us to remove repeat? It seems like we'd love to do it, but I'm guessing there's a good bit of legacy code. Is there any reason we can't, say, change the version number to 2.0.0 to indicate a breaking change and incentivize people to upgrade by letting them know that type checking is now available?

The only thing we really need to do is have the next release be a 2.x release so that we’re following semantic versioning. It’s not much of a hassle to drag along repeat until we’re actually ready to make that change though 👌

This way, we can continue shipping incremental changes in 1.x without having to manage multiple, major versions or anything.

theengineear · 2024-10-16T05:27:38Z

.github/workflows/test.yaml

@@ -12,6 +12,8 @@ jobs:
          registry-url: 'https://registry.npmjs.org'
      - run: npm ci
      - run: npm run lint
+      - run: npm run type
+      - run: git diff --exit-code


Rather than add a “build” step. We now have a CI step that will fail if a diff is generated from running npm run type. This is closer to the flow I think we want. I.e., rather than need to run a build to use the types… assert that the types were properly generated before allowing the merge.

theengineear · 2024-10-16T05:30:46Z

tsconfig.json

+    //  generated-js back to ts… it also seems to be the case that it will help
+    //  IDEs get to the right _js_ when using types. See
+    //  https://www.typescriptlang.org/docs/handbook/declaration-files/dts-from-js.html
+    "declarationMap": true,


After re-reviewing this. I think it’s actually most correct to emit a source map.

If you’re using the JS on it’s own — the source map is inert (since our JS is running as-is).

If you’re using the TS declaration file, the source map will hint IDEs to route you back to the original JS.

This was tricky to wrap my head around at first since I typically think about these source mappings as a way to go from the generated JS back to the original TS — but, it seems like you can leverage them to hint IDEs to do the opposite as well.

theengineear · 2024-10-16T05:33:15Z

package.json

    "/demo",
    "/test",
    "/etc"
  ],
  "devDependencies": {
    "@netflix/eslint-config": "^3.0.0",
+    "eslint-plugin-jsdoc": "^50.3.1",


After using this for a little while — I feel like this is a pretty good solution. Open to other suggestions though. FWIW, I tried using TypeScript to generate errors here and I kept feeling like it wasn’t doing what I wanted it to do — could be that I had it set up wrong though!

theengineear · 2024-10-16T05:37:24Z

x-element.d.ts

+     */
+    /**
+     * A property value.
+     * @typedef {object} Property


I’ll be curious to see how tools like JSR auto-doc this stuff!

theengineear · 2024-10-16T05:42:31Z

^^ Just tested the new CI control. It does indeed fail the build if a diff is detected and it prints the diff.

theengineear · 2024-10-16T06:03:32Z

Fiddling around a bit with uploading to JSR — the docs are good in some ways (and rough around the edges in other ways). But, increasingly, it’d be cool to be generating our docs from the actual code itself.

theengineear · 2024-10-16T06:15:27Z

x-element.js

-   *       }
-   *     };
-   *   }
+   * ```js


JSDoc also has an @example tag… but this feels more natural to write code blocks like this and it appears to be supported by JSR.

klebba · 2024-10-18T20:43:03Z

package.json

    "bump": "./bump.sh"
  },
  "files": [
    "/x-element.js",
+    "/x-element.d.ts",
+    "/x-element.d.ts.map",


do we need to generate a source map?

klebba · 2024-10-18T20:50:24Z

deno.json

@@ -1,5 +1,12 @@
 {
  "name": "@netflix/x-element",
  "version": "1.0.0",
-  "exports": "./x-element.js"
+  "exports": {


this seems wrong to not explicitly export the src

JSR’s documentation is not great here. I’ve added back the other files, which causes JSR to flag us as “slow” and throw warnings. I think that’s ok for now though.

I’m going to follow along on jsr-io/jsr#494 — hopefully things will get better once our JSDoc-style path forward here is accepted.

klebba

awesome!

theengineear · 2024-10-18T22:17:19Z

FYI, I think JSR is gonna wrongly flag this package as “slow” until jsr-io/jsr#494 is resolved. I think this should all work eventually though 👌

Previously, the “x-element.d.ts” file was hand-curated, which was prone to falling out of date. Because the TypeScript team manages JSDoc, it’s fairly straightforward to properly “type” your JS with JSDoc and output declarations which can be used by folks writing TypeScript — all of that without needing to actually author TypeScript ourselves.† Additionally, tools like JSR will build basic documentation for libraries which author JSDoc comments, so some wins there. Finally, to ensure best practices for our JSDocs, we enable some recommended rules from the eslint plugin. † One goal of “x-element” is to be _highly_ portable, it’s why no dependencies exist and why the “x-element.js” file can be used _verbatim_ without a build step.

theengineear · 2024-10-18T22:33:09Z

Here’s a screen grab of an rc publish to JSR I ran locally (a more zoomed out view). Again, not perfect, but definitely getting there.

theengineear · 2024-10-18T22:34:11Z

Going to get this merged in — I’ll play around with some rc releases to make sure everything still works as expected 👌

theengineear commented Oct 9, 2024

View reviewed changes

Kelketek reviewed Oct 11, 2024

View reviewed changes

theengineear force-pushed the generated-type-declarations branch from b69eb1f to f9d3bad Compare October 16, 2024 05:24

theengineear commented Oct 16, 2024

View reviewed changes

theengineear force-pushed the generated-type-declarations branch from f9d3bad to 71fa53b Compare October 16, 2024 05:35

theengineear commented Oct 16, 2024

View reviewed changes

theengineear force-pushed the generated-type-declarations branch from 71fa53b to 5fc6d6e Compare October 16, 2024 05:40

theengineear force-pushed the generated-type-declarations branch from 5fc6d6e to 79f945c Compare October 16, 2024 05:43

theengineear force-pushed the generated-type-declarations branch from 79f945c to 4018e8d Compare October 16, 2024 06:14

theengineear commented Oct 16, 2024

View reviewed changes

klebba reviewed Oct 18, 2024

View reviewed changes

klebba previously approved these changes Oct 18, 2024

View reviewed changes

theengineear dismissed klebba’s stale review via 7605866 October 18, 2024 22:18

theengineear force-pushed the generated-type-declarations branch from 4018e8d to 7605866 Compare October 18, 2024 22:18

theengineear force-pushed the generated-type-declarations branch from 7605866 to 54c9d2b Compare October 18, 2024 22:23

theengineear merged commit ad56dc1 into main Oct 18, 2024
1 check passed

theengineear deleted the generated-type-declarations branch October 18, 2024 22:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Generated type declarations #189

Generated type declarations #189

theengineear commented Oct 9, 2024 •

edited

Loading

theengineear Oct 9, 2024

theengineear Oct 9, 2024

theengineear Oct 9, 2024

theengineear Oct 9, 2024

Kelketek Oct 11, 2024 •

edited

Loading

theengineear Oct 16, 2024

theengineear Oct 9, 2024

Kelketek Oct 11, 2024

theengineear Oct 16, 2024 •

edited

Loading

theengineear commented Oct 9, 2024

Kelketek Oct 11, 2024

theengineear Oct 16, 2024

theengineear Oct 16, 2024

theengineear Oct 16, 2024

theengineear Oct 16, 2024

theengineear Oct 16, 2024

theengineear commented Oct 16, 2024

theengineear commented Oct 16, 2024

theengineear Oct 16, 2024

klebba Oct 18, 2024

klebba Oct 18, 2024 •

edited

Loading

theengineear Oct 18, 2024

klebba left a comment

theengineear commented Oct 18, 2024

theengineear commented Oct 18, 2024

theengineear commented Oct 18, 2024

		@@ -1,69 +1,239 @@
		export class XElement extends HTMLElement {

		@@ -0,0 +1 @@
		{"root":["./x-element.js"],"version":"5.6.3"}

Generated type declarations #189

Generated type declarations #189

Conversation

theengineear commented Oct 9, 2024 • edited Loading

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

Kelketek Oct 11, 2024 • edited Loading

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

theengineear Oct 16, 2024 • edited Loading

Choose a reason for hiding this comment

theengineear commented Oct 9, 2024

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

theengineear commented Oct 16, 2024

theengineear commented Oct 16, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

klebba Oct 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

klebba left a comment

Choose a reason for hiding this comment

theengineear commented Oct 18, 2024

theengineear commented Oct 18, 2024

theengineear commented Oct 18, 2024

theengineear commented Oct 9, 2024 •

edited

Loading

Kelketek Oct 11, 2024 •

edited

Loading

theengineear Oct 16, 2024 •

edited

Loading

klebba Oct 18, 2024 •

edited

Loading