Convert to ES6 that is supported on Node 4, commonjs modules and remove Babel #8
Conversation
| const Referencer = require('./referencer').default; | ||
| const Reference = require('./reference').default; | ||
| const Variable = require('./variable').default; | ||
| const Scope = require('./scope').default; |
There was a problem hiding this comment.
Can you use imports like require('./foo') here instead of require('./foo').default? Then the modules could use module.exports = Foo instead of module.exports.default = Foo.
There was a problem hiding this comment.
I was planning to do a style clean up PR at the end that would include this. Wanted to keep changes in each PR to a minimum but can change it now if people would prefer
There was a problem hiding this comment.
I don't really have a strong opinion either way, but I think it makes more sense to change this here since it's a direct result of migrating from ES6 modules.
| import { Syntax } from 'estraverse'; | ||
| import Map from 'es6-map'; | ||
| const Syntax = require('estraverse').Syntax; | ||
| const Map = require('es6-map'); |
There was a problem hiding this comment.
I think this import is unnecessary now since Map is included in Node 4. (That can be changed in a separate PR if necessary though.)
There was a problem hiding this comment.
@not-an-aardvark That is planned as a separate PR
| constructor(scopeManager, upperScope, block) { | ||
| super(scopeManager, 'class', upperScope, block, false); | ||
| } | ||
| } | ||
|
|
||
| module.exports = { | ||
| "default": Scope, |
There was a problem hiding this comment.
Since this isn't using modules anymore, it might be better to use module.exports.Scope instead of module.exports.default.
|
Looks good. Nothing too drastic which is nice. I would remove the check node.body commit until the other pull request is approved. |
This removes Babel, converts to commonjs modules and removes other ES6 features not supported on Node 4. I think it was all destructuring.