Refactor to ES6 #29
Refactor to ES6 #29
Conversation
…ode shouldn't be a part of repository.
| "modules": true | ||
| }, | ||
| "rules": { | ||
| "no-console": 1, |
There was a problem hiding this comment.
String literals instead of numeric shortcuts should be preferred.
|
As a general rule, moving to ES6 and especially ES6 modules is great and sensible step. Some parts of this should be revised though. Regarding tests, a small effort was made to have browser based instrumented testing for whatever active backend someone has, but that did not really get off the ground. |
| export const VERSION = '0.0.10' | ||
|
|
||
| export const digidoc_mime = 'application/x-digidoc' | ||
| export const digidoc_chrome = 'TokenSigning' |
There was a problem hiding this comment.
These should not see light outside of their actual implementations. The idea of modules is to keep things modular and self-contained. In fact, proliferation of "digidoc" throughout the codebase should be avoided or kept minimal.
| export const NO_IMPLEMENTATION = 'no_implementation' | ||
| export const NOT_ALLOWED = 'not_allowed' | ||
|
|
||
| export const localServiceURL = 'https://local.ria.ee' |
There was a problem hiding this comment.
Same here. What is local.ria.ee, "spotilocal.com"?
There was a problem hiding this comment.
Actually it is a bigger topic I would like to discuss.
Has local servlet as a pluginless backend been discussed? What are the reasons that contradict to this option?
So actually it currently should stay in a separate branch.
vellotis
force-pushed
the
master
branch
2 times, most recently
from
097815e to
b9522de
Compare
- Promise polyfill added. PanthomJS somewhy has it missing on Windows - Stubable mocks implemented for DigiDocPlugin and DigiDocExtension to test backend integration without needing them to be in place - Few bugs found and fixed
# Conflicts: # .travis.yml # bower.json # package.json
|
Tests added for different backends. |
|
Looking forward, it might be best to split it into separate commits, that:
And then separately:
For the API enhancements, some adjustments could be made at the same go, like making it explicit that getCertificate() is supposed to read getSigningCertificate(). Also, auth() does not need certificate as an input, this is implicitly dealt by the native part and returned in the JWS token. Whoever implements authentication, shall have to re-do quite a lot of the backend code as well, so picking up any newer API would be sensible at the same transition. |
|
And maybe a little more ad-hoc "internal api" for backend integration development would be nice to have, as a set of a few developer notes. |
The overall logic is mostly left in place but implemented as ES6. Babelify is used to transpile it to ES5 to make it compatible with all browsers.
Previously promises weren't used correctly. Promises by nature should support async usage. By just wrapping a synchronous logic into a
new Promise(function(resolve, reject) { /* synchronous logic */ })is not enough..auth(...)method has been created which in the future could comply with https://github.com/martinpaljak/x509-webauth/wiki/WebAuth.One thing that this PR doesn't cover is tests. The tests have been updated to pass for given refactoring but doesn't really cover much. ie. Every plugin/extension interface should have a mock to test against. So the same test suite should be ran for all interfaces.
A downside of this PR is a size of compiled hwcrypto.js which is 39.3KB and 16.3KB for minimized version which are almost 4x bigger than for previous ES5 implementation. But I don't think it is not a problem in terms of improved and improving internet speeds.
Manually tested with following browsers: