-
Notifications
You must be signed in to change notification settings - Fork 4
refactor as library using static type-checking #9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
dckc
wants to merge
38
commits into
marcsAtSkyhunter:master
Choose a base branch
from
dckc:flowtype
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mismatch with es6 specs discovered using flow
rather than ambient
Move them closer to other functions that need similar authority.
- import powerful objects only
if (require.main == module), i.e. only when this module
is invoked from the command line
- pass access to files, network, randomness explicitly
rather than using ambient authority
- to makeConfig(), sslOptions(), makeSturdy(), makeApp()
- .flowconfig, jshint declarations
- annotations for exports from caplib, saver
- name interface types in interfaces/types.js
- borrow es6 Proxy declaration from typescript
- adapt misc. node.js API types from typescript
- tweak commandArgs loop in argMap to not mutate
strings to numbers in-place
- tweak copyRecurse isDirectory test to avoid nulls
- tweak asId to make statically evident that
it can't return null
- use single assignment for newContext initialization
- explicitly check target[method] for null since flow
doesn't appreciate that it's surrounded by try/catch
While we're at it, get tool support for consistent style from eslint:
- indent 4 spaces. no tabs. no trailing space
- one true brace style with the single-line exception
- except for makeSaver, where emacs got confused
- double quotes for strings
- for unused args, use _x name style
- prune rawSend dead code
relax some of eslint's defaults:
- console logging is OK
|
I re-indented the code, but in a separate commit. |
- reduce filesystem access given to makeSaver
- ReadAccess, WriteAccess, SyncAccess give access to a specific path
- just one FileSystem type from node.js
- server.js: factor run() out of main()
- use ReadAccess for capper.config, ./ssl/*
- no need for readFileSync in sslOptions
- refactor reviverToMaker, reviverToUIPath as
as Reviver.toMaker and Reviver.sendUI
- sendUI takes an express Response, which carries authority
to open files by path name
- never mind note about express being nearly powerless
- log errors in showActor
|
This branch now has a complete draft of library packaging (#3). |
use .done() to raise exceptions
not just the main page
|
I just realized I haven't been maintaining the mocha tests. Better tend to that... |
- let ezSaver() convenience routine require anything
- use __dirname in makeReviver() to isolate from
caller's directory
- move io builders (fsReadAccess ,...) to saver.js
- pass saver to run() rather than dbfile
- pass crypto.randomBytes explicitly to makeUnique - use ezSaver convenience routine in tests
consolidate routes using /* and req.params[0]
in flow: Jul 18, 2016: 84a1a24@zacharygolba Add Proxy to core declarations somewhere before node 7.1, the --harmony-proxies option went away; https://github.com/tvcutsem/harmony-reflect says node groks as of 6.0.
goal is to log Error traces
- tighten up static types a bit
In particular, the `has(target, prop)` API.
- SealerPair:
- generic parameters
- nullable return from unseal
- import Promise from `q` to get `.done()` method
- add method arg to `Reviver.sendUI` decl
- the property in the Id type is `@id`, not `id`
- declare return type of vowAnsToVowJSONString
$ flow version
Flow, a static type checker for JavaScript, version 0.57.3
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I find static type-checking with flow invaluable for refactoring JavaScript. I actually worked up the
no-ambientbranch with type annotations in place and then took them out to submit the pull request.In order to minimize diffs, I didn't re-indent the code. I'm interested to know what you think of incorporating this into the project first.