megaserg / gototab-chrome-extension

Chrome extension analogous to Sublime/Atom Command Palette
3 stars 2 forks source link

Flux-inspired refactor #21

Closed megaserg closed 9 years ago

megaserg commented 9 years ago

/cc @vors

vors commented 9 years ago

I cannot compile it :( I follow appveyor.yml steps and run tsc --out .\out\gototab\FuzzySearch.js .\gototab\FuzzySearch.ts, but get this error:

tsc : D:\dev\gototab-chrome-extension\gototab\FuzzySearch.ts(200,7): error TS1008: Unexpected token; 'statement' expected.

D:\dev\gototab-chrome-extension\gototab\FuzzySearch.ts(212,1): error TS1008: Unexpected token; 'module, class, interface, enum, import or statement' expected.

In general, it seems incorrect, that build step compiles just one ts file. Even if this ts file includes all other files, it still seems, incorrect, because FuzzySearch.ts is definitely not a root file for project.

Can you, please clarify?

vors commented 9 years ago

+I cannot make build from AppVeyor zip package work.

megaserg commented 9 years ago

Okay so the problem was related to how the TypeScript modules depended on each other and how they were exported.

My initial approach was to define module in every .ts file, all definitions would go inside this module and I would put export for declarations that I wanted available globally. Like this PubSub.ts:

module pubsub {
  export foo = function(...) {...}
}

Then on the callsite, e.g. in Model.ts, I would put a "reference tag" at the top of the file (e.g. /// <reference path="PusSub.ts" />), and call exported functions like pubsub.foo(...);. But the responsibility of providing the needed dependencies at runtime (on a browser page) would be on me. Also, a minor issue is that I couldn't use module from PubSub.ts by any other name than pubsub, which was hardcoded in the module itself.

PubSub.ts would compile to a JS file like this:

var pubsub = {};
(function(pubsub) {
    pubsub.foo = function(...) {...} 
})(pubsub);

And the usage of it in Model.js would look like (apparently ported verbatim from Model.ts): pubsub.foo(...);

As you see, nothing is explicitly exported apart that pubsub variable is defined in whatever (global?) context. Nor were the dependencies specified, apart from comment /// <reference path="PusSub.ts" />, which is a no-op in the JS. So on a webpage, everything happened to work properly if I included the scripts on the right order. But in the test there was a problem. The test used NodeJS's require("./out/FuzzySearch.js"), but FuzzySearch.js didn't export anything, it just defined a local variable fuzzy (the name of the module)!

So I read the docs and found that it's possible to define functions on the top-level of the file, without having a module declaration. The callsite would use syntax similar to NodeJS's: import pubsub = require("./PubSub");. When compiled with --module commonjs, this provided the expected behavior for the test; naturally, the exported functions in JS now looked like:

var foo = function(...) {...}
exports.foo = foo;

and the callsite looked like:

var pubsub = require("./PubSub");
pubsub.foo(...);

But this broke the browser behavior. exports and require are only known to NodeJS, the browser needed something else. The alternative to --module commonjs is --module amd, generating JS files suitable for use with RequireJS. As much as I discouraged myself from using any libraries in this project, ultimately I made an exception for this one. A (questionable) benefit is that I don't have to manage dependencies manually now: as long as I tell RequireJS where my main code is, it includes the dependencies automatically and transitively. So here we are, with passing test and a library to make it all work.

megaserg commented 9 years ago

As for your compilation error, looks like it complains about the const keyword. It's introduced in EcmaScript 6, and TypeScript has a partial support for it. At least when I compile with either 1.4.1 or 1.5-alpha, I don't get any errors.

You're right, it makes sense to compile all files at the build step. One option is to use the "root" file popup.ts, all dependencies will be compiled automatically. Another is to use a wildcard, but I got an error when trying to do this:

tsc --module commonjs --outDir .\out .\gototab\*.ts

error TS6053: File 'gototab/*.ts' not found.
Command exited with code 2

Last time I used Windows, it supported wildcards like this. What did I do wrong?

vors commented 9 years ago

It's not about windows, but about tsc. You can probably rewrite it with powershell like

ls .\gototab\*.ts | % {tsc --module commonjs --outDir .\out $_.FullName}

But I would be pretty happy with

tsc --module commonjs --outDir .\out .\gototab\popup.ts
megaserg commented 9 years ago

Could it be because of the backward slashes instead of normal slashes?