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

issue/2824 Switch to rollup from requirejs #2923

Merged
merged 7 commits into from
Oct 15, 2020
Merged

issue/2824 Switch to rollup from requirejs #2923

merged 7 commits into from
Oct 15, 2020

Conversation

oliverfoster
Copy link
Member

@oliverfoster oliverfoster commented Sep 23, 2020

#2824
This version does not have the react elements, it is merely to fix the currently broken build process in v5.

Will provide an almost seamless migration away from build-side requirejs and AMD modules to ES6 modules.

Facilitates

  • future proof
    • import and export directives
    • beyond ES8 code
  • development improvements
    • much faster compilation and recompilation times on all grunt tasks, this is as a cache is now generated for the build process
    • supports larger code bases
    • move towards supporting intellisense in code editors
  • backward-compatibility
    • ES6 modules are reregistered in the browser as AMD modules, so var Adapt = require('core/js/adapt'); etc will still work in the console and for other plugins
    • we can now rework Adapt to ES6 modules whilst retaining support for the existing AMD modules
    • still ie11 compatible

Downsides

  • External modules such as libraries/mediaelement-fullscreen-hook.js, which are required by internal modules and require internal modules are now considered circular dependencies and will break the loading process. A temporary fix is included until that module can be moved back into the js folder
  • Jumping to class function line numbers from the code inspector in the debugger is out of order as the define/require directive conversions moves the line numbers
  • Currently this will only work with transpilation back to ES5 as calling an ES6 constructor function without the new keyword is not allowed - which is an essential part of the way Backbone constructs classes.

Added

Removed

  • requirejs as bundler in grunt
  • babel grunt task is now integrated into the rollup process
  • requirejs config.js file has been moved to the javascript grunt task config

Fixed

  • QuestionView has two different return statements, so cannot resolve to one export default directive

Testing

  • Use master of all plugins by running:
git clone https://github.com/adaptlearning/adapt_framework 2824
cd 2824
git checkout issue/2824
adapt devinstall && npm install
grunt dev

Example

Shows fast compilation times, import/export, debugging and require functioning with an ES6 module in the client-side:

rollup-es-modules2

Copy link
Contributor

Choose a reason for hiding this comment

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

👀

src/core/js/adapt.js Outdated Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

👀

@chris-steele
Copy link
Contributor

Looks fantastic, only issue I saw was a childViews reference runtime error with master branch of adapt-contrib-boxMenu, but works fine using latest release of said menu. 👍

@oliverfoster
Copy link
Member Author

@moloko Could you +1 when you get a mo?

Copy link
Contributor

@moloko moloko left a comment

Choose a reason for hiding this comment

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

code 👓

@oliverfoster oliverfoster merged commit 98ebb14 into master Oct 15, 2020
@oliverfoster oliverfoster deleted the issue/2824 branch October 15, 2020 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants