Add rollup, eslint, and matches polyfill #54
Conversation
| "plugins": [ | ||
| "external-helpers" | ||
| ] | ||
| } |
| @@ -0,0 +1,3 @@ | |||
| rollup.config.js | |||
| dist/**/*.js | |||
| node_modules/**/*.js | |||
There was a problem hiding this comment.
newline? I'm seeing this a lot lately - does VS Code strip EOF newlines by default or something?
There was a problem hiding this comment.
I'm not sure. Maybe there's a setting I can enable. I'll look
| this._observer = new MutationObserver(this._onMutation.bind(this)); | ||
| this._observer.observe(this._rootElement, { attributes: true, childList: true, subtree: true }); |
There was a problem hiding this comment.
Why remove these spaces? Does ESLint insist on it? Is there an option to make it not do that?
There was a problem hiding this comment.
It's part of the Google ESLint rules:
https://github.com/google/eslint-config-google/blob/master/index.js#L246
"off" or 0 - turn the rule off
"warn" or 1 - turn the rule on as a warning (doesn’t affect exit code)
"error" or 2 - turn the rule on as an error (exit code is 1 when triggered)
src/inert.js
Outdated
| @@ -109,7 +110,9 @@ class InertRoot { | |||
| * @param {Node} startNode | |||
| */ | |||
| _makeSubtreeUnfocusable(startNode) { | |||
| composedTreeWalk(startNode, (node) => { this._visitNode(node); }); | |||
| composedTreeWalk(startNode, (node) => { | |||
| this._visitNode(node); | |||
There was a problem hiding this comment.
Hm, remove {}s instead of spreading out over 3 lines?
src/inert.js
Outdated
| @@ -170,7 +173,9 @@ class InertRoot { | |||
| * @param {Node} startNode | |||
| */ | |||
| _unmanageSubtree(startNode) { | |||
| composedTreeWalk(startNode, (node) => { this._unmanageNode(node); }); | |||
| composedTreeWalk(startNode, (node) => { | |||
| _throwIfDestroyed() { | ||
| if (this.destroyed) | ||
| throw new Error("Trying to access destroyed InertNode"); | ||
| throw new Error('Trying to access destroyed InertNode'); |
There was a problem hiding this comment.
Oh no, can we go back to double quotes?
There was a problem hiding this comment.
Ok let's leave it with single quotes.
| @@ -298,9 +303,12 @@ class InertNode { | |||
| return this._destroyed; | |||
| } | |||
|
|
|||
| /** | |||
| * Throw if user tries to access destroyed InertNode. | |||
There was a problem hiding this comment.
Isn't this fairly redundant with the method name?
There was a problem hiding this comment.
yes. the ESLint rules require a JSDoc comment so I just added one in haste.
There was a problem hiding this comment.
Totally fair. Maybe just "Once an InertNode is obsolete, accessing any of its members throws an exception. This should never happen in practice." We can update it later tho.
| @@ -298,9 +303,12 @@ class InertNode { | |||
| return this._destroyed; | |||
| } | |||
|
|
|||
| /** | |||
| * Throw if user tries to access destroyed InertNode. | |||
There was a problem hiding this comment.
Totally fair. Maybe just "Once an InertNode is obsolete, accessing any of its members throws an exception. This should never happen in practice." We can update it later tho.
|
The README still refers to inert.min.js |
|
good catch, thank you |
Fixes #51
Kind of a big PR, sorry about that.
This switches our build over to using Rollup instead of Gulp. That means we can easily work with ES modules now (if we wanted to break the polyfill into smaller modules we could).
I'm also importing the dom-matches polyfill for IE/Edge support and producing a UMD build which will work standalone or with module loaders like browserify.
Also added ESLint so we could have more consistent PRs. I'm not currently linting the test directory because there were a handful of issues with mocha I didn't feel like resolving at the moment. Kinda want to land this PR first :)
I also killed the
inert.min.jsbuild for now because Uglify has issues with module syntax. There's other tools that will do the right thing but I wanted to try doing them in a different PR.@alice PTAL
@vcorr FYI