Skip to content
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

Move to using const and let #37

Open
matatk opened this issue Aug 1, 2017 · 4 comments
Open

Move to using const and let #37

matatk opened this issue Aug 1, 2017 · 4 comments
Assignees
Milestone

Comments

@matatk
Copy link
Owner

matatk commented Aug 1, 2017

This will be possible when PhantomJS 2.5 is out: ariya/phantomjs#14506

@matatk matatk self-assigned this Aug 1, 2017
@matatk
Copy link
Owner Author

matatk commented Sep 24, 2017

Have moved from PhantomJS to Karma, running the browsers for real (using headless Chrome on Travis). The tests and most of the main code have now been moved to using const and let, however...

Can't fully move to using ES6 until #38 is done, because currently everything's attached to window using var. (Before moving to NPM scripts, "exports.js" handled this, but the current build doesn't use it, and it doesn't seem time-efficient to fix that until after I have grokked how to properly re-assmenble the separated-out flies for production; using uglify's --wrap option looks like the right way.)

Update: rollup should be used when moving to ES6 modules.

@matatk
Copy link
Owner Author

matatk commented Oct 16, 2017

This can't be fully implemented until I've found the idiomatic way to do exposing things for test, and for wrapping all the internal API stuff for minification/production.

@matatk matatk added this to the MVP milestone Oct 16, 2017
@matatk
Copy link
Owner Author

matatk commented Oct 18, 2017

Current constraints (using spec/AudioChart.spec.js as an example):

  • Jasmine can spy on getAudioContext() when it's declared as a var because it's on window. But I want to declare as const.
  • If I declare it as const then, if I could also run each (source script, test spec) pair of scripts in a separate environment, then I could easily stub out getAudioContext()—but Karma doesn't support this; all source files are included at once in the browser window, so getAudioContext() is already declared and I can't stub it out, as that's a redeclaration of a const.

matatk added a commit that referenced this issue Oct 22, 2017
* Resign, for now, to #37 being an issue for testing.
* Add test for HTML Table options.
* Start adding tests for updating options.
@matatk
Copy link
Owner Author

matatk commented Nov 12, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant