[WIP] Externals

[DeMoorJasper]

"externals": {
  "react": "externalReact"
}

const React = require('react');

Adds this as a dependency

module.exports = externalReact;

SIDENOTE: This changes the resolvers rootPackage logic, now it gets the rootPackage inside bundler and adds it to the options object. This could potentially add a few ms to the ipc transfer times

Closes #144

[codecov-io]

Codecov Report

Merging #1216 into master will decrease coverage by 0.42%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #1216      +/-   ##
==========================================
- Coverage   88.28%   87.86%   -0.43%     
==========================================
  Files          77       78       +1     
  Lines        4355     4383      +28     
==========================================
+ Hits         3845     3851       +6     
- Misses        510      532      +22

Impacted Files	Coverage Δ
src/visitors/externals.js	100% <100%> (ø)
src/Bundler.js	89.47% <100%> (+0.09%)	⬆️
src/Resolver.js	100% <100%> (ø)	⬆️
src/assets/JSAsset.js	94.85% <100%> (+3.18%)	⬆️
src/utils/syncPromise.js	73.68% <0%> (-26.32%)	⬇️
src/assets/StylusAsset.js	69.41% <0%> (-17.65%)	⬇️
src/visitors/fs.js	75% <0%> (-6.25%)	⬇️
src/visitors/globals.js	92.3% <0%> (-3.85%)	⬇️
src/assets/GraphqlAsset.js	100% <0%> (ø)	⬆️
... and 6 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bb7f4aa...b0d9d5a. Read the comment docs.

[fathyb]

I'm not sure const React = React works though :

var React = {foo: 'bar'}

// Uncaught ReferenceError: React is not defined
void function() {
  const React = React

  console.log('React', React)
}()

// logs "React undefined"
void function() {
  var React = React

  console.log('React', React)
}()

Couldn't we just replace the react module with a simple module.exports = React or module.exports = externalReact? It's just an idea, I didn't look into details, but it'll be compatible with both ES6 and CommonJS and doesn't need a supplemental AST traversal.

[DeMoorJasper]

@fathyb Ow didn't think about that, that'll make it way easier.
I'll update the PR to that strategy soonish

[DeMoorJasper]

@fathyb Just realised there isn't a way to add an asset based on content atm.

[fathyb]

@DeMoorJasper ah, had same problem for the Angular integration and its virtual fs. Alternatively we could just internally handle it in Asset.js.

parcel/src/Asset.js

Lines 60 to 74 in bb7f4aa

	async getDependencies() {
	if (
	this.options.rendition &&
	this.options.rendition.hasDependencies === false
	) {
	return;
	}
	await this.loadIfNeeded();
	if (this.contents && this.mightHaveDependencies()) {
	await this.parseIfNeeded();
	await this.collectDependencies();
	}
	}

It could be implemented using something like :

 async getDependencies() { 
   if ( 
     this.options.rendition && 
     this.options.rendition.hasDependencies === false 
   ) { 
     return; 
   }

   if(isExternal(this.name)) {
     this.contents = `module.exports = ${getExternal(this)}`

     // No need to parse dependencies or transform the AST
     return
   }
  
   await this.loadIfNeeded(); 
  
   if (this.contents && this.mightHaveDependencies()) { 
     await this.parseIfNeeded(); 
     await this.collectDependencies(); 
   } 
 } 

I don't know if changing the content on the fly doesn't have any side-effects (content hash, cache...). Thoughts?

[DeMoorJasper]

@fathyb Seems a bit hacky, not sure how to resolve this in a proper way.
Perhaps we could introduce virtual fs to parcel?

[fathyb]

@DeMoorJasper Totally for it! 👍 I could also bring performance improvement if it's abstracted enough to build a in-memory fs like webpack-dev-server does.

[mohsen1]

Can we do this inline system for now and when in memory fs is in place optimize it later?

contents = `module.exports = ${getExternal(this)}`

[DeMoorJasper]

@mohsen1 Feel free to open up a PR that implements this.