POC/WIP: port to CommonJS #269
Conversation
|
👍 |
|
looks good - as discussed. adding an issue to test Some apprehension about |
| @@ -0,0 +1,17 @@ | |||
| var BannerPlugin = require('webpack/lib/BannerPlugin'); | |||
| var version = require('./package.json').version; | |||
| var webpack = require('webpack'); | |||
|
Why not es6 modules? Handlebars.js using it, for example |
|
none of our codebases use es6 modules. commonjs is widespread and not going away any time soon. |
|
@necolas but es6 transpilers can compile it to commonjs, or amd or standalone with change in build command one param, look at handlebars source, it is really awesome example of es6 module system. |
|
yeah i know, but we write all our code as commonjs. |
|
Is the idea here that a new major version would ship using CommonJS instead of RequireJS? |
|
Yes I hope so. And available on npm. |
| [ES5-shim](https://github.com/kriskowal/es5-shim) and | ||
| [jQuery](http://jquery.com) – and use an AMD module loader like | ||
| [Require.js](http://requirejs.org/) or | ||
| You will have to reference [jQuery](http://jquery.com) and use a CommonJS build |
There was a problem hiding this comment.
need to add webpack here
|
Changes to jasmine-flight to support a switch to commonjs: flightjs/jasmine-flight#44 |
|
@angus-c is something blocking this PR? |
|
@alexgorbatchev AFAIK it's still a WIP |
|
Would want more review of the changes to the test extensions and the consequences that might have on testing your app. Wasn't so keen on having to let the karma preprocessor/files know about every commonjs file in your dependency tree. |
|
Does it make sense to do a major semver change on this broad change? Technically API doesn't change here, but it's large enough change and it would be nice not to scare folks away by "sneaking" it under more liberal semver requirements using |
| @@ -6,7 +6,10 @@ VERSION = `node -pe "require('./package.json').version"` | |||
| clean: | |||
| @ rm -rf $(BUILD_DIR) | |||
|
|
|||
| standalone: clean | |||
| setup: | |||
There was a problem hiding this comment.
do this in a separate PR so we can apply it now; not related to the commonjs port
|
looks good |
* Use the commonjs preprocessor * Use npm-installed jQuery
|
the commonjs stuff has been in limbo for a while and isn't a requirement of getting flight registered with npm. so instead, we could publish the UMD build to npm (using a |
|
Closing. Will leave the branch in place if there's interest in picking it up in the future. |
|
Too sad that @necolas closed this PR. I had hoped that one day I could use FlightJS with Browserify. |
|
You can use Flight and CommonJS right now if you swap browserify for
|
|
Added npm support f462336 I published |
|
@andrewk Thank you for the tip. I'm gonna give it a try. |
Working proof-of-concept to have a look at.
Sane diff without whitespace changes: https://github.com/flightjs/flight/pull/269/files?w=1