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 Replaced requirejs with rollup for module bundler #2827

Closed
wants to merge 15 commits into from

Conversation

oliverfoster
Copy link
Member

@oliverfoster oliverfoster commented Jul 3, 2020

#2824
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 are reregistered in the browser as AMD modules, so var Adapt = require('core/js/adapt'); etc will still work
    • 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

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

var Logger = window.Logger = function() {
  this.logArr = [];
  this.registeredViews = [];
};

Example

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

rollup-es-modules2

Other

See a variant with added typescript support https://github.com/adaptlearning/adapt_framework/tree/issue/2824-ts with added command npm run check-types, it's a fair bit slower than the babel-only variety because it has to get processed by two compilers.

See a variant with added react support #2829 - I am very keen to get this in, it would mean we could ditch jQuery for DOM modification and we'd get a lot more flexibility as views will automatically rerender themselves on model changes.

@oliverfoster oliverfoster self-assigned this Jul 3, 2020
@oliverfoster oliverfoster requested a review from moloko July 3, 2020 11:03
Copy link
Contributor

Choose a reason for hiding this comment

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

👀

@moloko
Copy link
Contributor

moloko commented Jul 8, 2020

See a variant with added typescript support https://github.com/adaptlearning/adapt_framework/tree/issue/2824-ts with added command npm run check-types, it's a fair bit slower than the babel-only variety because it has to get processed by two compilers.

Just to note that the TypeScript compiler should be able to transpile down to ES5 by itself, so there shouldn't be any need to run it through babel as well. Honestly though, don't worry too much about trying to include support for TypeScript - I think getting support for React in Adapt would be a far bigger win.

@oliverfoster
Copy link
Member Author

oliverfoster commented Jul 8, 2020

I also had to make two custom AST transformation which are really easy to do in babel, for amd to ES6 and for jsx templates. Hopefully by the time we've converted everything over to ES6, typescript will have matured to a level where making transformation plugins is as easy as it is in babel. If we limit the stack through rollup, babel and react then it leaves Typescript as a future possibility as the sole compiler.

I couldn't get it to transpile from ES6 > ES5, the --target option set to ES5 just told me that class isn't a thing when compiling. I am probably missing some properties. Perhaps --lib? https://www.typescriptlang.org/docs/handbook/compiler-options.html

@oliverfoster
Copy link
Member Author

oliverfoster commented Jul 8, 2020

Yea, I was missing --lib microsoft/TypeScript#25853

@oliverfoster
Copy link
Member Author

oliverfoster commented Jul 8, 2020

This was a reasonably good article on the subject: https://iamturns.com/typescript-babel/
Except that the proposed "easy" "lightning-fast" solution doesn't work. Specifically "@babel/proposal-class-properties" adds class properties after the constructor (and therefore after initialize)... which is a bit useless to us babel/babel#8510. Whereas the actual typescript compiler adds class properties, as expected, before the constructor, as it is in native backbone (backbone puts the values on the prototype but typescript puts them on the instance, same difference in practise).

I was kind of aiming for what I remember you wanted, which was typing on compile. So none of the options I could find really came together without a compromise but it did look promising a few times and was really exciting to see the intellisense light up properly in vscode - and boy did it light up, was amazing.

I think what I'd prefer to see is a proper typescript plugin for babel, not just a stripper.

@oliverfoster
Copy link
Member Author

closed in favour of #2829

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.

4 participants