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

jspm support #198

Closed
dpinart opened this issue Jun 28, 2016 · 25 comments · Fixed by #631
Closed

jspm support #198

dpinart opened this issue Jun 28, 2016 · 25 comments · Fixed by #631

Comments

@dpinart
Copy link

dpinart commented Jun 28, 2016

Is there any way to create a new project with the cli that uses jspm/systemjs as module loader?

@EisenbergEffect
Copy link
Contributor

Not at this time, no.

Consider that jspm and system.js are two separate things. We will probably add support for system.js first and consider jspm at a later date. jspm is the most problematic of all build options due to its stability and issues within enterprises. If you want jspm now, the best way to go is use our skeletons or set up a custom solution.

@XuJinNet
Copy link

System.js +1

@JeroenVinke
Copy link
Collaborator

SystemJS support

I think the best way to start working on this is to first create a new aurelia-cli project (with requirejs) and modify that project to work with systemjs. You'll probably want to follow these instructions to be able to make changes to the CLI easily. As soon as the project is working with SystemJS, we can teach the project creation part of the CLI to create this exact setup.

The CLI has both a bundler and a loader. For the bundling part we use amodro-trace which uses RequireJS internally to trace dependencies of an app. What is created by the bundler is a bundle that requirejs as well as systemjs should be able to load. So we don't need to modify the bundler part of the CLI, only the loader that is used.

I would start by editing the aurelia.json file (require -> systemjs, empty plugins array as they are requirejs specific, remove text plugin dependency, remove this line).

The way the current setup works is that the loader is in the vendor-bundle.js, which gets loaded through a script tag in index.html. So we need to make sure that the current bundler can bundle SystemJS, and if not what is preventing it from being bundled. The vendor-bundle is configured via aurelia.json to contain the loader configuration which can be found at the bottom of vendor-bundle.js:

image

This config is requirejs specific, so the cli should create a similar systemjs config. This configuration is created here based on the type of loader. Here is the code that creates the RequireJS config, so that's what we need to do here but then for SystemJS

cc @simonfox

@simonfox
Copy link
Contributor

simonfox commented May 13, 2017 via email

@simonfox
Copy link
Contributor

simonfox commented May 16, 2017

Right I've started to look at this @JeroenVinke and I've run into a couple of things and one problem I haven't been able to get to the bottom of...

  1. I don't think we can include the loader in vendor-bundle.js the same way it is with requirejs. Instead it needs to be loaded via a standard script tag. This may also mean bundle.prepend in aurelia.json would need to be added as standard script tags if those scripts need to be loaded before the loader - see my index.html

  2. I have what I think is close to the correct SystemJS config in this config.js I haven't made any changes to the CLI yet so I have included the bundle in the repo that has the implementation of _aureliaConfigureModuleLoader using that config. Pretty standard, one thing of note is that we need to enable the RequireJS-like API to load the bundles (NOTE the browser will take a while to load to these linked lines L21431-L21432)

  3. Now here is the problem that I need some help with....the bundles are being loaded, but the modules contained are not being used. There are still requests generated for each individual file:
    Network tab
    so just to make things load I set up the transpiler on the loader but the issue is described by loading AMD modules from r.js bundled file systemjs/systemjs#947 I just haven't been able to decipher what @guybedford means by separately import the file as that looks like what I am doing here

Are you able to help at all? or @AStoker ?

@JeroenVinke
Copy link
Collaborator

Great work @simonfox! I can look into this tomorrow

@AStoker
Copy link
Contributor

AStoker commented May 16, 2017

Just taking a quick peek, I'm kinda loaded up at work right now.
But something stuck out. This line (https://github.com/simonfox/cli-systemjs-loader/blob/b859e36ced617cea534e209613928b2970be8944/scripts/vendor-bundle.js#L21446) seems to reference the bootstrapper at a higher level. I wonder if that's throwing off all the imports to request files at a different path than they're defined as, and therefore trying to get the files again.

@simonfox
Copy link
Contributor

Ah sorry @AStoker ....in the latest commit that is not how it is configured (the mappings are at a consistent level)...I had been trying a bunch of variations of the mapping and published it inconsistent like that by mistake. Thanks for taking a look though, will keep this thread updated if I make any further progress.

@simonfox
Copy link
Contributor

simonfox commented May 17, 2017

@JeroenVinke @AStoker to check the config, I installed systemjs-builder and built true systemjs bundles using that config (only change I made was to replace src with lib as I transpiled the es6 to that folder for bundling). And it works (NOTE I didn't inline the html templates as part of the bundling as I just wanted to do a quick test so these are still loaded individually).....

network

Another option would be to modify CLI to use aurelia-bundler when the loader is SystemJS (if the translation from aurelia.json bundles to aurelia-bundler config is easier than just translating directly to SystemJS Builder Module Expression Tree and using that builder directly)?

@JeroenVinke
Copy link
Collaborator

Do I need a modified version of the CLI to run https://github.com/simonfox/cli-systemjs-loader?

Also, could you share the bundle you created with systemjs-builder? I'm curious to see what it tells the systemjs loader vs what we tell it

@simonfox
Copy link
Contributor

@JeroenVinke

Right I've found the issue....SystemJS needs the bundle define to contain full mapped path i.e.
define("aurelia-bootstrapper", ....
needs to be
define("node_modules/aurelia-bootstrapper/dist/system/aurelia-bootstrapper.js", ...

Do I need to look into amodro writeTransforms?

@simonfox
Copy link
Contributor

simonfox commented May 18, 2017

@JeroenVinke OK ignore my last post....I feel dumb!

If you replace the generated _aureliaConfigureModuleLoader in vendor-bundle.js with the following it will just work.

function _aureliaConfigureModuleLoader(){
  window.define = SystemJS.amdDefine;
  window.require = window.requirejs = SystemJS.amdRequire;
  
  SystemJS.config({
    "baseURL":"src"
  });
}

Because the require.js bundle uses normalized names for define there is no mapping required (at least for this simplist of cases)!

The only issue I haven't solved now is how to load templates. app.html is still being loaded as an individual file, not from the app-bundle. This is the reason SystemJS.config needs baseURL to be set, everything else is loaded fine the SystemJS.config call - the amdDefine setup is all that is needed.

@JeroenVinke
Copy link
Collaborator

JeroenVinke commented May 18, 2017

Nice work @simonfox!

I've been digging and think it's working now. The app-bundle should be loaded by SystemJS. So what I did was comment out the app-bundle script tag from index.html. Then I've used the following config in vendor-bundle:

function _aureliaConfigureModuleLoader(){
  window.define = SystemJS.amdDefine;
  window.require = window.requirejs = SystemJS.amdRequire;
  
  SystemJS.config({
    "baseURL":"src",
    map: {
      text: '../node_modules/systemjs-plugin-text/text.js',
      "app-bundle": "../scripts/app-bundle.js"
    },
    bundles: {
      "app-bundle": ["app", "main", "environment", "resources/index"]
    }
  });
}

Note that you need SystemJS's text loader: npm install github:systemjs/plugin-text --save. In aurelia.json, change "text" in the dependencies section to "systemjs-plugin-text",.

Could you confirm that this resolves the issue?

If so, then you could proceed as follows (but feel free to take another path). First clone and link the CLI so that you can make changes to the CLI (see these instructions).

  • we'll want this function to generate the SystemJS config i've posted above. It looks very similar to RequireJS, so see if you can use bits of that
  • it would be cool if we could get plugins to work. The stub property of a plugin indicates whether or not to include the loader plugin into the bundle. For example, if you set stub of the text loader to true, it will not end up in the bundle and therefore you cannot fetch text resources at runtime. It will be used at build time though. So the stub property allows you to reduce the bundle size by not including loader plugins in the bundle that you're not going to be using at runtime
  • when everything is working, we should modify au new to include the requirejs option.

@simonfox
Copy link
Contributor

simonfox commented May 18, 2017

@JeroenVinke that works!...we can actually remove baseUrl as well

SystemJS.config({
    map: {
      "text": 'node_modules/systemjs-plugin-text/text.js',
      "app-bundle": "scripts/app-bundle.js"
    },
    bundles: {
      "app-bundle": ["app", "main", "environment", "resources/index"]
    }
  });

there is one other manual change I made in app-bundle that we will need bundle generation to do for systemjs - I changed the define moduleId for the template from require text!app.html to systemjs app.html!text

Do you know if I can easily change amodro output for plugin loaded modules? SystemJS has a pluginFirst config option, that could help but would mean there would be changes required in aurelia/loader-default

@JeroenVinke
Copy link
Collaborator

@simonfox
Copy link
Contributor

@JeroenVinke perfect!

I'll get onto the actual cli changes tomorrow.

@simonfox
Copy link
Contributor

@JeroenVinke right I've got the guts of this done in https://github.com/simonfox/cli/tree/feature/systemjs-support

I'll get onto the au new changes next....

Let me know if you have any initial comments on what I have already done!

@EisenbergEffect
Copy link
Contributor

@simonfox We really appreciate you working on this! Keep up the great work!

@simonfox
Copy link
Contributor

simonfox commented May 19, 2017

@JeroenVinke I've made most of the au new changes...

Last thing is index.html. Should I just change the resources folder to have a require.index.html and a system.index.html and then output the correct one to the project index.html (using ProjectItem)?

@AStoker
Copy link
Contributor

AStoker commented May 19, 2017

Echoing what the Eise' Man said, thanks so much for your help on this @simonfox! Really appreciate you working with him on this too @JeroenVinke.
Power Team, go! 💪 💥

@JeroenVinke
Copy link
Collaborator

Nice work @simonfox. I wonder if the builder pattern may be a bit too much. While the SystemJS and RequireJS config are very similar, those two may be the only ones that could benefit from the builder pattern. What do you think?

@JeroenVinke
Copy link
Collaborator

Last thing is index.html. Should I just change the resources folder to have a require.index.html and a system.index.html and then output the correct one to the project index.html (using ProjectItem)?

Yes, that sounds fine to me.

I'm curious though, why can't systemjs be bundled?

@simonfox
Copy link
Contributor

@JeroenVinke yes good point, I'm not familiar with Webpack but just had a quick look and the config is a bit different. The steps I've added are very granular leaving most of the template in place, maybe if the template had chunkier steps it might work. I will roll it back for now.

Have just moved systemjs to the bundle and can confirm it does work (now we have the necessary config correct). Two other things to decide

  • the startup SystemJS.import("aurelia-bootstrapper") import can be moved out of index and appended to the end of vendor-bundle after the definition of _aureliaConfigureModuleLoader and it all works. That means we can remove that from index.html
  • that just leaves the extraneous data-main="aurelia-bootstrapper" on the vendor-bundle script tag. It doesn't hurt, however it may cause confusion for new users if we leave it there?

Thoughts?

@JeroenVinke
Copy link
Collaborator

@simonfox having a requirejs.index.html and systemjs.index.html should be fine. That would allow you to put SystemJS.import('aurelia-bootstrapper") in index.html

simonfox added a commit to simonfox/cli that referenced this issue May 20, 2017
Enables support for SystemJS as module loader including new project setup via `au new`.

closes aurelia#198
simonfox added a commit to simonfox/cli that referenced this issue May 26, 2017
Enables support for SystemJS as module loader including new project setup via au new.

closes aurelia#198
simonfox added a commit to simonfox/cli that referenced this issue May 26, 2017
Enables support for SystemJS as module loader including new project setup via au new.

closes aurelia#198
simonfox added a commit to simonfox/cli that referenced this issue May 27, 2017
Enables support for SystemJS as module loader including new project setup via au new.

closes aurelia#198
simonfox added a commit to simonfox/cli that referenced this issue May 27, 2017
Enables support for SystemJS as module loader including new project setup via au new.

closes aurelia#198
@AStoker
Copy link
Contributor

AStoker commented Jun 12, 2017

Great job @simonfox and @JeroenVinke for getting this in!!! 👏 🎆

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants