WIP add TypeScript #7
Conversation
… intellisense in VS Code or TypeScript-capable editors)
| @@ -4,11 +4,11 @@ import { initVisualizer } from './visualizer/80sgrid.js'; | |||
| import { initEditor } from './editorcontroller.js'; | |||
|
|
|||
| customElements.define('app-javascriptmusic', | |||
| class extends HTMLElement { | |||
| class JavaScriptMusic extends HTMLElement { | |||
There was a problem hiding this comment.
Giving the class a name improves the intellisense output in editors.
| constructor() { | ||
| super(); | ||
|
|
||
| const shadowRoot = this.attachShadow({mode: 'open'}); | ||
| this.shadowRoot = this.attachShadow({mode: 'open'}); |
There was a problem hiding this comment.
Element.shadowRoot is nullable, but by assigning to this.shadowRoot in the constructor it makes the type system not require a null check further down below because it knows it is definitely not null now (plus removed the unused variable).
| window.AudioContext = window.webkitAudioContext; | ||
| /** @type {any} */ | ||
| const win = window | ||
| window.AudioContext = win.webkitAudioContext; |
There was a problem hiding this comment.
I used any here so TS won't complain about window.webkitAudioContext being a non-existent property. We don't care much about the polyfill types, the end user doesn't care about it.
| } | ||
|
|
||
| if(typeof AudioWorkletNode !== 'function') { | ||
| console.log('No audioworklet support - using polyfill'); | ||
| window.AudioWorkletNode = function(context, processorName) { | ||
|
|
||
| window.AudioWorkletNode = /** @type {AudioWorkletNode} */(AudioWorkletNodePolyfill) |
There was a problem hiding this comment.
This syntax is a type cast. The parentheses are required here. /** @type {any} */(AudioWorkletNodePolyfill) would be shorter and would also work (we don't care much about types here anyways).
|
|
||
| /** | ||
| * @param {any} context | ||
| * @param {any} processorName |
There was a problem hiding this comment.
As a quick escape hatch, just give things type any. Then it is as if we wrote plain JS anyways.
I'm just getting it set up, I'll update the types a little later...
| @@ -114,3 +123,6 @@ if(typeof AudioWorkletNode !== 'function') { | |||
| window.audioWorkletProcessors[name] = new processorClass(); | |||
| } | |||
| } | |||
|
|
|||
| // For TypeScript to recognize the file as a "module". | |||
| export {} | |||
There was a problem hiding this comment.
Due to legacy TypeScript functionality from before ES Modules were a thing, any file without an import or export statement is not considered a module. If the file is not considered a module, the app.js file is not able to import it (type error on that import line). Adding the no-op export {} line marks the file as a module in TS eyes.
|
I am running into a problem with VS Code and the language service not updating the error state of the intellisense. microsoft/TypeScript#37955 It's probably due to a combination of no I have a feeling the issue will probably go away after adding some more options and making the setup more like standard TypeScript projects. When you checkout this branch, and open it in VS Code, you should see errors in many places (because many types are missing, and the Then, when you start editing any files, you should notice the red squigglies do not update. Hopefully someone other than me can reproduce the issue. In the meantime, I'll update the project to be more like a normal TS project (but only with type checking, no build), and see if the issue goes away. |
|
Regarding moving away from global vars, we could start by storing everything on the As yet another PR idea for later, it would be nice to organize the repo so that top-level files are meta files related to repo and code management, then the code in few top-level folders. As an example, my repo at https://github.com/lume/glas has meta files at the top, and everything else in the |
|
Thanks @trusktr ! This looks really useful. I like the idea of moving away from global vars too, good to make it self-contained. |
|
Yaaay, they fixed the bug I mentioned. microsoft/TypeScript#37955 I'll get back to this soon. That bug was making types in plain JS very unusable. |
|
TypeScript 3.9 is out. Looks like I can swing back around to this pull request soon, without that aforementioned issue being in the way! |
|
Great :-) I've added some tests since last time (added mocha and karma). As the project is growing it's good to have some tests to prevent things from breaking. |
|
Nice!! Hey Peter, are you on Discord yet? AssemblyScript moved to Discord: https://discord.gg/YMW3wXT I'd like to circle back to this soon, and have some questions. Would be great to chat (easier). |
|
Yeah, I’m already in there. My handle is psalomo |
WIP, not to be merged yet, but we can use this PR to discuss and see progress.
There's no transpilation, only type checking and improved intellisense in VS Code or TypeScript-capable editors.
This is also a good process for me to get familiar with the code.
This PR will focus only on adding type checking and intellisense (f.e. when viewing the code in VS Code, etc), without modifying the functionality of the code. Everything will work exactly as it does now.
For a following PR, one thing that I would like to change is to remove the use of global variables for configuration, and instead require options to be passed as attributes or props to the
app-javascriptmusicelement (or other related APIs). This will make it easy to embed the app anywhere in any site, without polluting global state; i.e. makes it more self-contained.For yet another PR, I'd like to work on making the code more consumable (f.e. set it up to be publishable on NPM and easy to import and use in any project). In particular, I'd like to decouple the existing UI from the audio engine (JS + AS stuff) to make it easy to import and use the JS/AS APIs standalone without pulling in the code editor UI, without the visualization, etc. Decoupling the code by removing global variables and enforcing inputs and outputs between the components will help with this.