Tram-One, now with Types! Typescript! v11.0.0 #175

JRJurman · 2021-10-21T03:35:13Z

Summary

In an effort to get the advantages of type safety, this PR migrates the source code (and most of the tests) to Typescript.

This gives the project more type safety, and allows our consumers to build projects with type safety (if they are using Typescript). If they aren't using Typescript they still will get proper type annotations when working with compatible editors (such as Visual Studio Code).

Changes

All the changes listed below are detailed in the comments, treat these as a high level breakdown.

Bump Major - v11.0.0

In reality, this might not be breaking for JS projects, but there are a significant number of changes (including the removal of some safety checks for JS and addition of checks for TS). Anyone using Typescript would start getting compile time errors / warnings, so for that reason alone really it should be considered a breaking change.

Main Changes

Remove manual type checking in the project
Remove manual JS Doc annotations
types.ts file for typescript type definitions

Auxiliary Changes

Using Prettier instead XO
Expand ...args for more clear parameter definitions
Fix file JSDoc blocks to not be associated with a variable
Fixed Document TreeWalker to properly filter elements
Removed server-side rendering support

Testing Changes

Added new test for verifying root-defined effects
Created Add browser testing for cases that jsdom can't handle (notably scroll position) #176 (we lost code coverage for elements that have scroll position, in reality, we probably never actually had coverage for these, and it was being ignored)

Artifacts: typescript...typescript-artifacts

Note about the artifacts

For some reason, CircleCI slowed down between the last master run and this changeset. This was confirmed with a dummy PR which had similar slowdowns (with no changes).

Checklist

Post Merge Checklist

Update Website

JRJurman · 2021-10-22T04:03:44Z

.circleci/config.yml

@@ -31,7 +31,7 @@ jobs:
          fingerprints:
            - "64:3e:d4:d8:4b:95:68:79:d3:3b:ab:b1:5c:fa:2d:3d"

-      - run: npm install
+      - run: npm ci


I think this project originally predated npm ci, but now that it exists, we should be using that, and not regenerating the lock file on every PR.

JRJurman · 2021-10-22T05:05:53Z

A lot of the runtime checks can actually be removed now that we verify the return type of Tram-One Components

JRJurman · 2021-10-24T02:01:21Z

.gitignore

-# SSL certificates
-.ssl
-


This was most certainly an artifact of when we had the website hosted from this repo, since that is no longer a thing, we can remove this

JRJurman · 2021-10-24T02:03:29Z

.prettierrc.js

+module.exports = {
+	useTabs: true,
+	singleQuote: true,
+	printWidth: 120,
+};


Using prettier instead of xo.

The prettier config is more known to work with typescript, and xo was breaking too much.

Eventually I'd like to rip out the eslint config for tram-one projects as it is today. It really should be something that validates tram-one things (e.g., I would love to lint template spacing, and hook placement). Right now, it's just silly opinions, and that doesn't really help anyone.

This includes using semicolons (which sadly blows the number of lines changed for the project super high). ASI has burned me enough times (read, more than 0 times), and I think with the new Typescript syntax, it's easier if we are on the same standard as everyone else.

JRJurman · 2021-10-24T02:20:41Z

integration-tests/integration.test.js

+	it('should process effects on the root node', async () => {
+		// start the app
+		const { container } = startApp();
+
+		// verify that it (eventually) has loaded: true
+		await waitFor(() => {
+			expect(getByText(container, 'Root Loaded: true')).toBeVisible();
+		});
+	});


New test, we didn't really have something to validate this, but we did have code in the project that would have caused this to fail and nothing else.

JRJurman · 2021-10-24T02:23:09Z

integration-tests/test-app/sub-mirror-input.ts

+ * component to test url parameters
+ */
+export default () => {
+	const mirrorable = useGlobalStore('mirrorable-input') as InputObject;


The type isn't actually required if you pass in a default value. The fact that this option exists is cool, and possible because of function overloading.

JRJurman · 2021-10-24T02:24:31Z

integration-tests/warnings.test.js

-	it('should warn if a component does not return anything', () => {
-		expect(() => startBrokenApp('empty')()).toThrowError('Tram-One: expected component to return an Element, instead got undefined. Verify the component is a function that returns DOM.')
-	})
-
-	it('should warn if a component does not return an element', () => {
-		expect(() => startBrokenApp('non-dom')()).toThrowError('Tram-One: expected component to return an Element, instead got string. Verify the component is a function that returns DOM.')
-	})
-
-	it('should warn if a component returns an array', () => {
-		expect(() => startBrokenApp('array')()).toThrowError('Tram-One: Sorry, Tram-One does not currently support array returns. Wrap components in an element before returning.')
-	})


aforementioned removed tests

the other ones in this file still exist because they are more runtime dependent. Technically we should be able to catch the "warn if a hook is called outside of a component context", but that requires building lint or typescript rules around that. Nothing obvious or trivial was available online on how to do this.

JRJurman · 2021-10-24T02:28:11Z

package.json

@@ -1,6 +1,6 @@
 {
  "name": "tram-one",
-  "version": "10.1.11",
+  "version": "11.0.0",


Major bump. In reality, this might not be breaking for JS projects, but anyone using Typescript would start getting compile time errors / warnings, so for that reason, it should be breaking.

JRJurman · 2021-10-24T02:32:13Z

src/build-mounting-container.ts

+/**
+ * Function to determine (or create) the element that we will mount our tram-one app onto
+ * @param target either a CSS selector, or HTMLElement to attach the component to.
+ * This elememnt should be initially empty.
+ *
+ * @returns the container, now with a div that tram-one can manage
+ */
+export default (target: ElementOrSelector): HTMLElement => {


DocStrings no longer have types in them. These should be inherited from the Types in the method definition / implementation.

JRJurman · 2021-10-24T02:36:44Z

src/dom.ts

+		const hookedTagFunction = (props: Props, children: Children) => {
+			// push a new branch onto the working key so any values that need to be unique among components
+			// but consistent across renders can be read
+			const stringifiedProps = JSON.stringify(props);
+			const newBranch = `${tagName}[${stringifiedProps}]`;
+			pushWorkingKeyBranch(TRAM_HOOK_KEY, newBranch);
+
+			// increment branch so that we have a unique value (in case we are rendering a list of components)
+			incrementWorkingKeyBranch(TRAM_HOOK_KEY);
+			const uniqueBranch = copyWorkingKey(TRAM_HOOK_KEY);
+
+			// create a tag function that has the args passed in
+			const populatedTagFunction = () => {
+				// reset working key so we have the correct place when starting a new component
+				restoreWorkingKey(TRAM_HOOK_KEY, uniqueBranch);
+
+				return tagFunction(props, children);
+			};


Previously this function used (...args) instead of (props, children) in method definitions. Potentially at the time I didn't know what this method definition would be, or wanted it to be more flexible, or maybe it was really really early on when component methods had a ton of things in them. Either way, it looks like this now, and should be easier to understand and more explicit.

JRJurman · 2021-10-24T02:38:11Z

src/effect-store.ts

@@ -1,33 +1,27 @@
-/**
+/*


In some places I had inadvertently (or lazily) use /** which makes blocks more readily, but gets consumed as docstrings for the next variable, which was not the intention.

JRJurman · 2021-10-24T02:40:50Z

src/mount.ts

+	// this sadly needs to be wrapped in some element so we can process effects
+	// otherwise the root node will not have effects applied on it
+	const renderedApp = html`<div><app /></div>`;


This is the code that was previously untested. Ideally this wouldn't exist, and potentially we should annotate it / change it to be more clearly intended for tram-one (with a different tag, or an id). However, any more additions to this would push us away from just getting rid of it altogether.

~~We should look to change this into a Document Fragment 🤔~~
Never mind, probably runs into the same issues.

JRJurman · 2021-10-24T02:43:46Z

src/mutation-observer.ts

+	const nodeFilterForTramOneComponent = { acceptNode: isTramOneComponent };
+	const componentWalker = document.createTreeWalker(node, NodeFilter.SHOW_ELEMENT, nodeFilterForTramOneComponent);


Funny story, previously, this was implemented wrong. createTreeWalker takes in an object with a key acceptNode. Typescript caught this, I'm not sure how this was working before, and sadly it doesn't seem to have improved performance. Either way, it's correct now 👍

JRJurman · 2021-10-24T02:45:25Z

src/namespace.ts

+	window['tram-space'] = {};
+};
+
+export const buildNamespace = <NamespaceStore>(constructor: () => NamespaceStore) => {


In order to have the get set and setup functions all produce the same Type, we now have one generic function whose type is inferred by the constructor that is used.

JRJurman · 2021-10-24T02:47:49Z

src/observable-hook.ts

+ *
+ * It has a similar interface to React's useState
+ */
+export default <Store extends StoreObject>(key?: string, value?: Store): Store => {


Previously this function verified the type of the string and value, now that can be verified with Typescript! JS users will still get annotations in Typescript capable editors.

JRJurman · 2021-10-24T02:50:49Z

src/observe-tag.ts

+				// we'll assume that the element is an HTMLInputElement, in reality other kinds of elements will be caught here,
+				// but that's fine, since they have null as selection attributes, and setting them to null is fine
+				const activeElement = document.activeElement as HTMLInputElement;


Pretty much what it says in the comment. This is because Typescript (and somewhat inaccurately so) complains that "Element" doesn't have the selection attributes (e.g. selectionStart), in reality, these will be null, which I guess for Typescript is just as bad, but for us, that is fine.

JRJurman · 2021-10-24T02:52:52Z

src/process-effects.ts

+ *
+ * These are later processed by the mutation-observer, and cleaned up when the node is removed by the mutation-observer.
+ */
+export default (tagFunction: TramOneComponent) => {


Previously this function verified that the tagFunction returned an Element, but now we get that with Typescript!

JRJurman · 2021-10-24T03:17:36Z

src/use-global-store.ts

+/**
+ * @name useGlobalObservable
+ * @link https://tram-one.io/#use-global-observable
+ * @description
+ * Hook that stores global state and makes it accessible in the entire app.
+ *
+ * If the subfield of an object, or element of an array is updated
+ * it will cause only the components that are dependent on that value to update.
+ *
+ * @param key a unique string to write and read the global value
+ * @param defaultValue the default value to start the store at
+ *
+ * @returns the store to interact with.
+ */
+function useGlobalStore<Store extends StoreObject>(key: string, defaultValue: Store): Store;
+/**
+ * @name useGlobalObservable
+ * @link https://tram-one.io/#use-global-observable
+ * @description
+ * Hook that stores global state and makes it accessible in the entire app.
+ *
+ * If the subfield of an object, or element of an array is updated
+ * it will cause only the components that are dependent on that value to update.
+ *
+ * @param key a unique string to write and read the global value
+ *
+ * @returns the store to interact with.
+ */
+function useGlobalStore(key: string): unknown;
+/** Implementation of the two function definitions */
+function useGlobalStore(key: string, defaultValue?): any {


Function overloading! The first two definitions (19 & 33) are the two possible method interfaces, and the third definition (35) is the more generic version that is used for the code implementation. This allows us to have a version of the method that knows the return type (in this case, inferred from the defaultValue). In cases where you don't pass a default value, you have to specifically say what the type is.

JRJurman · 2021-10-24T19:02:58Z

README.md

+### Is Tram-One for Javascript or Typescript?
+
+Both! While the source code and type checking exist in Typescript, smart editors (such as Visual Studio Code), will make use of
+the Typescript annotations regardless of what language you work in!
+


I'd love to make a more general FAQ and Principles section in the Tram-One Website, but in the interim, this will have to do 😅

JRJurman · 2021-11-08T04:25:09Z

When playing around in a typescript project, people can use the following syntax to verify the component follows the correct interface:

import { registerHtml, TramOneComponent } from 'tram-one'

const html = registerHtml()

const AppHeader: TramOneComponent  = (props, children) => {
	return html`
		<h1 class="app-header">
			${children}
		</h1>
	`
}

export default AppHeader

JRJurman · 2021-11-08T22:11:35Z

docs/runkit_example.js

+// NOTE: This won't work since Tram-One does not (right now) support server-side rendering
+// This is on our backlog though, and if it is something that you would like, please vote / comment here:
+// https://github.com/Tram-One/tram-one/issues/181


Domino was removed, so this runkit example won't work, but I'm keeping it here because one day it hopefully will 😄

JRJurman · 2021-11-09T21:10:40Z

integration-tests/test-app/account.ts

+/**
+ * component to test url parameters
+ */
+const account: TramOneComponent = () => {


This is a pattern components can use, to verify that they follow the interface for a "TramOneComponent"

JRJurman · 2021-11-09T21:12:28Z

load-package.cjs

+// file needed for importing package-json
+// see https://rollupjs.org/guide/en/#using-untranspiled-config-files
+module.exports = require('./package.json');


basically what the comments say, this is required for loading the package.json in es modules.

JRJurman · 2021-11-09T21:13:30Z

package.json

+	"type": "module",
+	"exports": {
+		".": {
+			"import": "./dist/tram-one.mjs",
+			"require": "./dist/tram-one.cjs"
+		}
+	},


using types and export, this may not be totally consumed correctly until the next version of typescript, but we can future proof it 👍

JRJurman · 2021-11-09T21:14:58Z

src/dom.ts

+			// decorate the properties expected on TramOneElements (see node-names.ts)
+			tagResult[TRAM_TAG] = true;
+			// we won't decorate TRAM_TAG_REACTION, that needs to be done later when we observe the tag
+			tagResult[TRAM_TAG_NEW_EFFECTS] = tagResult[TRAM_TAG_NEW_EFFECTS] || [];
+			tagResult[TRAM_TAG_CLEANUP_EFFECTS] = tagResult[TRAM_TAG_NEW_EFFECTS] || [];


decorating all the properties of a tram one tag, so that they are available for sure

JRJurman · 2021-11-09T21:16:13Z

src/tram-one.ts

+export {
+	ElementOrSelector,
+	DOMTaggedTemplateFunction,
+	TramOneComponent,
+	StoreObject,
+	CleanupEffect,
+	Effect,
+	Props,
+	Registry,
+	UrlMatchResults,
+} from './types';


Exposing these types publicly, so that other Typescript projects can use them directly.

JRJurman · 2021-11-09T21:17:06Z

src/types.ts

+/**
+ * Type for when we can take a CSS Selector, or an HTML Element (mostly mounting).
+ */
+export type ElementOrSelector = [string | HTMLElement][0];


I'm not 100% sure about this pattern, but it allows consumers to understand what the library is looking for (rather than just saying "ElementOrSelector")

ethanjurman

Awesome stuff! Cannot wait for typescript magic!!

chtinahow

👍

Initial Pass at migrating to Typescript

dc2bf8c

JRJurman marked this pull request as draft October 21, 2021 03:36

JRJurman added 4 commits October 20, 2021 23:37

re-add lint (placeholder)

a43298e

migrate test app to ts as well

e56dc64

type definitions!

64dab27

npm ci in pipeline

1b76e38

JRJurman commented Oct 22, 2021

View reviewed changes

11.0.0

1ca3030

JRJurman changed the title ~~Initial Pass at migrating to Typescript~~ Tram-One, now with Types! Typescript! v11.0.0 Oct 22, 2021

JRJurman added 5 commits October 22, 2021 02:01

remove manual type checks

349d159

add eslint typescript config

839e459

fix lint, add comments to config, update test files to ts

029217e

cleanup jsdocs, more types, add unit test for root effect

fdcadee

use prettier

30f8370

JRJurman commented Oct 24, 2021

View reviewed changes

fix docstring, update dependency name in config

f18e844

JRJurman commented Oct 24, 2021

View reviewed changes

JRJurman added 2 commits October 24, 2021 14:51

clean up layout for tasks test component

7e8d7fd

Update README

c6842cf

JRJurman commented Oct 24, 2021

View reviewed changes

JRJurman added 4 commits October 25, 2021 01:49

more strict typing

c8a677e

formatting for test app

771d55e

Fix app by being dependent on loading

ca74d2a

fix performance test ts

31024f5

JRJurman marked this pull request as ready for review October 29, 2021 01:36

JRJurman added 5 commits October 30, 2021 01:21

use functions for node filter

8195f6b

typescript build config

da0e1fb

use parcel 2.0

74e2788

strictNullChecks, remove domino

2edd8c2

rearrange and expose types in main export

d7727f9

JRJurman added 2 commits November 8, 2021 13:51

strict ruleset, update test files to verify type

1df7179

change to type module

0968852

JRJurman mentioned this pull request Nov 8, 2021

Support server-side rendering #181

Open

update runkit example

2914e36

JRJurman commented Nov 8, 2021

View reviewed changes

JRJurman commented Nov 9, 2021

View reviewed changes

ethanjurman approved these changes Nov 10, 2021

View reviewed changes

chtinahow approved these changes Nov 10, 2021

View reviewed changes

JRJurman added 2 commits November 9, 2021 22:17

update eslint, remove logs, expose TramOneElement

eb88229

fixes for code coverage

adef448

JRJurman merged commit ce27626 into master Nov 10, 2021

JRJurman mentioned this pull request Apr 30, 2022

Update for v11 / typescript examples Tram-One/tram-one.github.io#22

Closed

JRJurman deleted the typescript branch May 15, 2022 21:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tram-One, now with Types! Typescript! v11.0.0 #175

Tram-One, now with Types! Typescript! v11.0.0 #175

JRJurman commented Oct 21, 2021 •

edited

Loading

JRJurman Oct 22, 2021

JRJurman commented Oct 22, 2021

JRJurman Oct 24, 2021

JRJurman Oct 24, 2021

JRJurman Oct 24, 2021

JRJurman Oct 24, 2021

JRJurman Oct 24, 2021

JRJurman Oct 24, 2021

JRJurman Oct 24, 2021

JRJurman Oct 24, 2021

JRJurman Oct 24, 2021

JRJurman Oct 24, 2021

JRJurman Oct 24, 2021

JRJurman Oct 24, 2021

JRJurman Oct 29, 2021 •

edited

Loading

JRJurman Oct 24, 2021

JRJurman Oct 24, 2021

JRJurman Oct 24, 2021

JRJurman Oct 24, 2021

JRJurman Oct 24, 2021 •

edited

Loading

JRJurman Oct 24, 2021

JRJurman Oct 24, 2021

JRJurman commented Nov 8, 2021 •

edited

Loading

JRJurman Nov 8, 2021

JRJurman Nov 9, 2021

JRJurman Nov 9, 2021

JRJurman Nov 9, 2021

JRJurman Nov 9, 2021

JRJurman Nov 9, 2021

JRJurman Nov 9, 2021

ethanjurman left a comment

chtinahow left a comment

		const nodeFilterForTramOneComponent = { acceptNode: isTramOneComponent };
		const componentWalker = document.createTreeWalker(node, NodeFilter.SHOW_ELEMENT, nodeFilterForTramOneComponent);

Tram-One, now with Types! Typescript! v11.0.0 #175

Tram-One, now with Types! Typescript! v11.0.0 #175

Conversation

JRJurman commented Oct 21, 2021 • edited Loading

Summary

Changes

Bump Major - v11.0.0

Main Changes

Auxiliary Changes

Testing Changes

Checklist

Post Merge Checklist

Choose a reason for hiding this comment

JRJurman commented Oct 22, 2021

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

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

JRJurman Oct 29, 2021 • 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

Choose a reason for hiding this comment

JRJurman Oct 24, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JRJurman commented Nov 8, 2021 • 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ethanjurman left a comment

Choose a reason for hiding this comment

chtinahow left a comment

Choose a reason for hiding this comment

JRJurman commented Oct 21, 2021 •

edited

Loading

JRJurman Oct 29, 2021 •

edited

Loading

JRJurman Oct 24, 2021 •

edited

Loading

JRJurman commented Nov 8, 2021 •

edited

Loading