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.

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

Use ES6 modules instead of AMD #45

Closed
Reinmar opened this issue Dec 2, 2015 · 3 comments
Closed

Use ES6 modules instead of AMD #45

Reinmar opened this issue Dec 2, 2015 · 3 comments
Assignees
Milestone

Comments

@Reinmar
Copy link
Member

Reinmar commented Dec 2, 2015

The discussion was started in ckeditor/ckeditor5-design#73 whether we shouldn't use ES6 modules rather than AMD. So far the discussion is not concluded, but there were three reasons why we started investigating using ES6 modules:

  • getting rid of hacks that we had to make AMD work,
  • ability to easily run the code in Node.JS (we used couple of hacks to adjust AMD to our needs, but these hacks were problematic when porting the code to different envs),
  • circular dependencies which with ES6 modules deal easily, but which were constant problem in the complex OT world (sure, you can always solve this by making architectural changes, but we've seen that these changes make the code worse, not better).

I'll conclude my research later on, because I found it very interesting (and not straightforward).

The plan for now is to use Babel to transpile ES6 modules to AMD (for browsers) or CJS (for Node) in a way which will also allow to use ES6 modules natively in the future.

@Reinmar
Copy link
Member Author

Reinmar commented Dec 23, 2015

I've been talking with PJ that we definitely need to start transpiling all the code (so including tests and test tools), because:

  • we will not be able to run tests on older browsers (we use ES6 features in the tests),
  • there's huge mess when part of the code is ES6 modules transpiled to AMD and the rest is clear AMD or even plain JS files.

As a second stage of this ticket we will add tests to the current pipeline.

@Reinmar
Copy link
Member Author

Reinmar commented Dec 23, 2015

Notes for me (I'll explain them later if I decide to go for this):

/dist
    /amd
        /ckeditor5
            /core
                /ui
                    view.js
                editor.js
            /classic-creator
                classiccreator.js
            load.js
        ckeditor.js

        /tests
            /_lib
                sinon.js
            /_tools
                tools.js
            /core
                /_tools
                    tools.js
                editor.js
                tests-tools.js
            /classic-creator
                /_tools
                    tools.js
                classiccreator.js
                tests-tools.js
            load.js
            tests-tools.js

/dist
    /amd
        ckeditor.js



## /tests/core/foo.js

import CKEditor from 'ckeditor5/ckeditor.js';
import load from 'ckeditor5/core/load.js';

...

## ckeditor.js

define( 'ckeditor5/core/editor.js', [ '../../ckeditor5/ckeditor.js' ], ... )

## index.html

require( 'ckeditor.js', ( CKEDITOR ) => {
    require( 'ckeditor5/core/editor.js', () => {

    } );
} );

@Reinmar
Copy link
Member Author

Reinmar commented Dec 30, 2015

It seems that I managed to transform the code into a set of ES6 modules with Gulp+Babel taking care of transpilation. Unfortunately, couple of things didn't work due to issues in other libraries:

Other than that, architecturally, there is one more step to do – transforming tests to ES6 modules as well. Without that we'll have multiple problems when referencing dev code in the tests and tests tools. I encounter serious issues when I wanted to load e.g. a piece of dev code in a test tool. Plus, we must make the test work in non-fully-ES6-compatible browsers anyway. So this will be an important step as well. Most likely I'll work on this right after fixing the watcher.

And the last step – documentation. I made a huge amount of changes in the code, changed some concepts and made some decision based on the previous research. All this needs to be transformed into docs. Otherwise, in a couple of weeks we will start making the same mistakes which I did along the road :D.

@Reinmar Reinmar mentioned this issue Dec 30, 2015
Merged
@Reinmar Reinmar modified the milestone: 0.1.0 Mar 4, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant