VIP: Migrate imports and exports from CommonJS to ES2015 #11690
Conversation
matticbot
added
OSS Citizen
[Size] L
jsnmoon
added
[Status] Needs Review
|
Looks sane, tho you might have to be careful with the But TBH, I don't know how to test VIP related stuff -- my changes to those files were part of a bulk change, and I'm not sure I even tested them myself or just asked @scottsweb to do that for me 😄 BTW @scottsweb -- is this VIP stuff inside Calypso being used at all? |
| var setTitle = require( 'state/document-head/actions' ).setDocumentHeadTitle, | ||
| sites = require( 'lib/sites-list' )(); | ||
| import { setDocumentHeadTitle as setTitle } from 'state/document-head/actions'; | ||
| const sites = require( 'lib/sites-list' )(); |
There was a problem hiding this comment.
A pattern that's used in a few places for imports like sites-list looks something like this:
import sitesFactory from 'lib/sites-list';
const sites = sitesFactory();Though the naming isn't entirely consistent throughout the code base (here, here & here for examples)
Might it be worth making that change in this PR or leaving for another PR that targets this pattern specifically?
There was a problem hiding this comment.
Hmm... let's punt that change for the next bulk change.
|
|
||
| /** | ||
| * Internal Dependencies | ||
| */ | ||
| var setTitle = require( 'state/document-head/actions' ).setDocumentHeadTitle, | ||
| sites = require( 'lib/sites-list' )(); | ||
| import { setDocumentHeadTitle as setTitle } from 'state/document-head/actions'; |
There was a problem hiding this comment.
Perhaps there's value in keeping setDocumentHeadTitle as-is vs aliasing (though I do see that either way it's out of the scope of this PR).
There was a problem hiding this comment.
I'm personally on the same page, but I refrained from making too many discretionary edits :)
|
@jsnmoon This PR needs a rebase |
|
Hmm, looks like we can close this since |
matticbot
removed
the
[Status] Needs Review
|
Haha, works for me! |
Part of #11688.
This change will allow us to leverage Webpack 2's tree shaking optimization for ES6 modules in the future. Please share your suggestions and comments!