-
-
Notifications
You must be signed in to change notification settings - Fork 255
ace bug #38
Comments
Ace does need some careful configuring... I'll see if I can check it out this week and suggest some configuration for it. |
@BuildingJarl nice work! That looks like it could be fixed with a shim on install:
Do post back if you make progress. |
@guybedford Thanks for the help, but unfourtunatly it doesnt seem to be working. This could also be because of my limited understanding. I have tried the following:
Both resulting in the wrong object being exported when installing ace normally
The following object is exported, which is correct: |
@BuildingJarl you can fix the object with |
@guybedford The ace editor calls this function: By using this function: This is the normal procedure for changing themes: |
Right, so Ace is trying to do its own module loading it seems, expecting an AMD environment. By default, SystemJS doesn't make the main code environment an AMD one ( Perhaps see if there is a way in the Ace API to "inject" the theme in and load the module manually - something like: // myace-with-clouds-theme.js
import ace from 'ace';
import clouds from 'ace/theme/clouds';
ace.injectTheme(clouds); // <--- what would this line be?
export default ace; |
Unfortunately I have not found a way to do what you suggested, but I managed to get the theme and the mode changed used a quick and dirty fix. All I did was copy the source from the theme-twilight file into the ace source. This is not a good solution and I will try to find another one. |
@guybedford I have found a different solution:
import ace from "ace-builds";
import "ace-builds/theme-monokai";
import "ace-builds/mode-html";
var editor = ace.edit("editor");
editor.setTheme("ace/theme/monokai");
editor.getSession().setMode("ace/mode/html"); One problem still remains: Ace requires an external script for a web-worker when changing the mode. I think this is for showing errors. |
@BuildingJarl this is great!! It looks like you've nearly worked this out. For the source mode worker, does it help if you make |
@guybedford No, unfortunately that doesn't work. But there is another way to get the webworker to load. Im not sure if this is the correct way of doing it though. In ACE the following line loads the worker
This issues a get request for the worker.js file. The get request doesnt know where the files is located, so we have to configure ACE's basepath. Like so:
Now, the web worker is able to load and no errors pop up in the console. The whole code being:
Result: |
AMAZING! This would be a really good argument for allowing an |
@guybedford Wondering if the workaround suggested by @BuildingJarl can be improved without having to directly reference the install path. Basically could we leverage jspm/systemjs so that
Becomes something like:
I'm not really sure if systemjs/jspm has an API method like that, or if it's possible (I tried briefly to find it in either API but figured it would be easier to ask you @guybedford ) |
You could try using |
This works perfect in development mode (where I have a filesystem), but in my production build I have a giant SFX, so I'm not sure how to workout setting a "basePath", if only ace had the option to pass in a web-worker object instead of a path |
@guybedford Just a quick follow up, I found a project dedicated to hacking ace.js to have inlined web workers. https://github.com/thlorenz/brace The package works in a development build no problem (without need of setting basePath). However I have run into a setback when building a SFX. The build executes, however it errors out on the first ace.define call. My guess is that Systemjs is building the subsequent calls in brace/index.js before the initial export and definition of ace itself. The exporting seems rather strange (to me) so its hard for me to debug. It looks like index.jx first executes an anonymous function namespaced to "ace" and then runs ace's proprietary ace.define for subsequent ace modules and extensions. Perhaps if my import syntax were different, or maybe there is a way in jspm/systemjs to specify order when bundling a given module? Any thoughts or pointers are greatly appreciated thank you. Link to brace/index.js |
I have made some progress. Stock brace works in production workflow (although since stock does not set mode, obviously the web worker does not load). From there, I globally shimmed the theme file as such:
This worked, giving me hope to do the same with the mode and worker:
Alas I am getting the ace is not defined error during the mode files System.registerDynamic call. @guybedford Any clue why the dependency global worked for the theme file but not the mode/worker file? Does Systemjs not allow declaring the same dependency twice? |
Success! the shim was perfect, however in the file in which i set the mode to javascript I needed to import the worker file (despite never explicitly using it). Here is the final code to load up ace:
Where every theme, extension, mode, worker needs to be shimmed in the config file:
Caveat
|
this worked for me: jspm install ace then... import ace from 'ace';
// get the path to the ace theme/mode assets- eg "http://localhost:9000/jspm_packages/github/ajaxorg/ace-builds@1.2.2.js"
let base = System.normalizeSync('ace');
// strip the trailing ".js" from the path:
base = base.substr(0, base.length - 3);
// configure ace with the base path
ace.config.set('basePath', base);
// now we're ready to use Ace
let editor = ace.edit(someElement);
editor.setTheme('ace/theme/monokai');
editor.getSession().setMode('ace/mode/javascript'); |
Huge props to @jdanyow for his approach. Since ace seems pretty set on using its own module loading, it seems that not trying to hack around it will lead to the most stable results in the long run. |
@jdanyow Thanks for you solution. But seems it's not working for extension. |
Hey this fixes my problem:
Thanks for the tips anyway. |
follow @MadMub 's example, the meta definition can be simplified to
works on jspm 0.16.39 systemjs 0.19.31. |
print
no found!
The text was updated successfully, but these errors were encountered: