Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Amd and commonjs to es6 #131

Closed
wants to merge 2 commits into from
Closed

Amd and commonjs to es6 #131

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f3c1bca
fix spacing in define
vmora Jul 6, 2016
6765237
move from AMD and CommonJS to ES6 modules
vmora Jul 6, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .babelrc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"presets": ["es2015"],
"plugins": ["transform-runtime"]
}
2 changes: 1 addition & 1 deletion doc/API_Doc/ApiGlobe.js.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ <h1 class="page-title">Source: ApiGlobe.js</h1>
*/


define('Core/Commander/Interfaces/ApiInterface/ApiGlobe', [
define( [
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this define needed?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an artifact of the sed script being used to transform "named defines" to "unnamed defines" prior to running amdtoes6 (which doesn't support named defines). The sed script modified that file, but amdtoes6 obviously didn't.

IMO, either we:

  • ignore it, leave the commit like that; we'll eventually remove the generated doc from master anyway
  • remove doc from master first, then rebase that change on top
  • exclude that file from the commit

'Core/Commander/Interfaces/EventsManager',
'Scene/Scene',
'Scene/NodeProcess',
Expand Down
9 changes: 8 additions & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@
"lint": "eslint \"src/**/*.js\" \"test/**/*.js\"",
"test": "npm run lint && npm run build",
"build": "webpack -p",
"start": "webpack-dev-server -d --inline --hot"
"start": "webpack-dev-server -d --inline --hot",
"prepublish": "babel -d dist/ src/"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will transform ES6 sources to ES5 with CommonJS imports, so it's a "legal" NPM module (see #122); it should target lib/ then, not dist/ (which contains the output from webpack).

main above should then be updated to point to lib/Main.js.

And .npmignore should ignore dist/ (we'll publish the ES5 sources to NPM, not the output from webpack), and .gitignore should also ignore lib/.

(see #44 where this was all done)

Copy link
Contributor

@tbroyer tbroyer Jul 7, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An alternative would be to use CommonJS for now and stick with ES5: no need for that prepublish calling babel, just update the main to point to src/Main.js and add dist/ to .npmignore.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking more about it: we don't follow NPM / Node.js rules for imports (non-relative names –i.e. those not starting with ./ or ../– are resolved against node_modules sibling folders walking up the hierarchy; so require("Core/Commander/Command") wouldn't work in "pure Node.js", it'd have to be either require("itowns2/lib/Core/Command/Command") to resolve to the file from an installed itowns2 NPM module; or require("./Command") –or import Command from './Command'– when called from our InterfaceCommander). See http://www.commonjs.org/specs/modules/1.0/ and https://nodejs.org/api/modules.html for the details.

Our current usage of referencing files by their absolute path “relative” to our src root only works because we configured Webpack that way, but it won't/wouldn't work if we publish our files to NPM and try to use the module from another project (if using Webpack or Rollup –and probably also Browserify–, one could configure it to make it work, but that's additional burden).

So I think we should just remove that prepublish script, and revisit things when we want to tackle the NPM issue (see #44).
(FWIW, I also think we should move to using relative import paths, like everyone else in the JS/Node.js land, but again, that's another matter that should be addressed in a distinct PR)

},
"repository": {
"type": "git",
Expand All @@ -29,6 +30,12 @@
"simd": "^2.0.0"
},
"devDependencies": {
"babel-cli": "^6.5.1",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm really not sure babel-cli is needed.

AFAICT, the babel executable is installed as part of babel-core, and babel-cli is meant to be installed globally (npm install -g babel-cli) so it's available on your command-line anywhere, and will "sniff" the current module to actually call the locally-installed babel (at node_modules/.bin/babel) if it exists; defaulting to the globally-installed one otherwise.

"babel-core": "^6.5.1",
"babel-loader": "^6.2.2",
"babel-plugin-transform-runtime": "^6.5.0",
"babel-preset-es2015": "^6.5.0",
"babel-runtime": "^6.5.0",
"eslint": "^2.5.3",
"eslint-loader": "^1.3.0",
"js-beautify": "^1.6.2",
Expand Down
60 changes: 28 additions & 32 deletions src/Core/Commander/Command.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,38 +4,34 @@
* Description: Cette object contient une commande à executer. Elle porte également les buffers résultants.
*/

define('Core/Commander/Command', [], function() {
function Command() {
//Constructor

this.name = null;
this.priority = 0.0; //Math.floor((Math.random()*100));
this.state = null;
this.inParallel = null;
this.inBuffers = null;
this.outBuffers = null;
this.paramsFunction = {};
this.processFunction = null;
this.async = null;
this.force = null;
this.type = null;
this.addInHistory = null;
this.source = null;
this.requester = null;
this.provider = null;

}

Command.prototype.constructor = Command;

/**
*/
Command.prototype.instance = function() {
//TODO: Implement Me

function Command() {
//Constructor

this.name = null;
this.priority = 0.0; //Math.floor((Math.random()*100));
this.state = null;
this.inParallel = null;
this.inBuffers = null;
this.outBuffers = null;
this.paramsFunction = {};
this.processFunction = null;
this.async = null;
this.force = null;
this.type = null;
this.addInHistory = null;
this.source = null;
this.requester = null;
this.provider = null;

}

Command.prototype.constructor = Command;

/**
*/
Command.prototype.instance = function() {
//TODO: Implement Me

};
};

return Command;
});
export default Command;
64 changes: 32 additions & 32 deletions src/Core/Commander/InterfaceCommander.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,49 +4,49 @@
* Description: Cette Classe construit une commande. Cette Command ensuite pousser dans une file d'attente.
*/

define('Core/Commander/InterfaceCommander', ['Core/Commander/ManagerCommands', 'Core/Commander/Command', 'when'], function(ManagerCommands, Command, when) {
import ManagerCommands from 'Core/Commander/ManagerCommands';
import Command from 'Core/Commander/Command';
import when from 'when';

function InterfaceCommander(type) {
//Constructor
function InterfaceCommander(type) {
//Constructor

this.managerCommands = ManagerCommands();
this.type = type;
this.managerCommands = ManagerCommands();
this.type = type;

}
}

InterfaceCommander.prototype.constructor = InterfaceCommander;
InterfaceCommander.prototype.constructor = InterfaceCommander;

/**
* @return {[object Object]}
*/
InterfaceCommander.prototype.buildCommand = function() {
//TODO: Implement Me
this._builderCommand();
};

InterfaceCommander.prototype.request = function(parameters, requester) {
/**
* @return {[object Object]}
*/
InterfaceCommander.prototype.buildCommand = function() {
//TODO: Implement Me
this._builderCommand();
};

var command = new Command();
command.type = this.type;
command.requester = requester;
command.paramsFunction = parameters;
command.layer = parameters.layer;
InterfaceCommander.prototype.request = function(parameters, requester) {

command.promise = new when.promise(function(resolve, reject) {
command.resolve = resolve;
command.reject = reject;
});
var command = new Command();
command.type = this.type;
command.requester = requester;
command.paramsFunction = parameters;
command.layer = parameters.layer;

//command.priority = parent.sse === undefined ? 1 : Math.floor(parent.visible ? parent.sse * 10000 : 1.0) * (parent.visible ? Math.abs(19 - parent.level) : Math.abs(parent.level) ) *10000;
command.promise = new when.promise(function(resolve, reject) {
command.resolve = resolve;
command.reject = reject;
});

command.priority = requester.sse ? Math.floor(requester.isVisible() && requester.isDisplayed() ? requester.sse * requester.sse * 100000 : 1.0) : 1.0;
//command.priority = parent.sse === undefined ? 1 : Math.floor(parent.visible ? parent.sse * 10000 : 1.0) * (parent.visible ? Math.abs(19 - parent.level) : Math.abs(parent.level) ) *10000;

this.managerCommands.addCommand(command);
command.priority = requester.sse ? Math.floor(requester.isVisible() && requester.isDisplayed() ? requester.sse * requester.sse * 100000 : 1.0) : 1.0;

return command.promise;
};
this.managerCommands.addCommand(command);

return command.promise;
};

return InterfaceCommander;

});
export default InterfaceCommander;
Loading