[ACE-2722] Clean, extensible import API for JavaScript resources Created: 09-Apr-12  Updated: 31-Jan-18

Status: Open
Project: Alfresco One Platform
Component/s: JavaScript API, Web Scripts and Surf
Affects Version/s: 4.0
Fix Version/s: 5.1

Type: Feature Priority: Major
Reporter: Mark Rogers [X] (Inactive) Assignee: Share Team
Resolution: Unresolved Votes: 16
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Attachments: PDF File AFaust Contribution Agreement.pdf     Text File Repo+Share-WebScript-Import.patch     Text File RhinoScriptProcessor-Import.patch     Text File Spring-JSScriptProcessor-Import.patch    

 Description   

Currently, script imports are handled by a preprocessor directive placed at the top of JavaScript files / content. Constituent scripts are merged into one only at runtime. Import resolution only allows for classpath and simple XPath expressions. This is limiting in the fact that

a) scripts with import directives are syntactically invalid and thus prevent support by JavaScript capable IDEs
b) no script can be imported dynamically, e.g. as the result of some evaluation
c) breakpoints for debugging can not be set / specified prior to the merging process, e.g. not before the script has been executed at least once

I propose the following changes to provide a JavaScript-based import API. This API additionally enables extensibility on the Repository side through a pluggable locator registry.

Some background information to the motivation for this improvement is available at my blog (http://axel-faust.de/?p=75&lang=en as well as http://axel-faust.de/?p=47&lang=en)



 Comments   
Comment by Axel Faust [ 09-Apr-12 ]

Patch files for proposed improvement.

Comment by Nick Burch [ 10-Apr-12 ]

Does the patch maintain backwards compatibility? i.e. do the old style <import tags still work?

It's a minor thing, but might you be able to re-do the patch following the Alfresco Coding Conventions? <http://wiki.alfresco.com/wiki/Coding_Standards> details them. That'll make the review process easier, and applying too (if accepted). Also, if you don't have a Contribution Agreement on file, and chance you could send one in? See <https://wiki.alfresco.com/wiki/Source_Code> for details.

Comment by Axel Faust [ 10-Apr-12 ]

Yes, the patch maintains backwards compatibility as it leaves the preprocessor code in place. It is even possible to mix the old style import with the new, as all JS will be run through the preprocessor regardless of import style. The Repo+Share-WebScript-Import.patch is a "secondary" patch for the convenience of bulk switching to the new style as the product should be the "good example" to follow for community developers.

Comment by Nick Burch [ 10-Apr-12 ]

Wonderful, thanks for the contributions, sounds great! Someone will need to do a fuller review before committing, hopefully that'll happen fairly soon (developer time permitting...)

Comment by Torsten Werner [ 07-May-14 ]

This issue is more than 2 years old. Are there plans to incoporate the change into Alfresco?

Comment by Axel Faust [ 07-May-14 ]

Torsten Werner: Not to my knowledge. That is why I created https://github.com/AFaust/alfresco-enhanced-script-environment as an addon.

Comment by Torsten Werner [ 07-May-14 ]

nice!

Comment by Petr Bodnár [ 05-Aug-14 ]

Haven't tried yet but it does look like a nice contribution! Maybe it's too complex to be reviewed and acknowledged by the Alfresco team? Or are they about to develop their own solution to this painful "import directive" problem?

Back in 2009 we have developed our own minimalistic solution, a modification of RhinoScriptProcessor which enabled at least a subset of what is (what I think) typically required from web scripts' developers:

a) use xpath:
<import resource="/app:company_home/app:dictionary/cm:webscripts/cm:com/.../cm:script.js">

This should be more useful than the default </Company Home/Data Dictionary/Web Scripts/includeme.js form because every Alfresco instance can have the "core" folders named in different language, can't it? (e. g. "Datový slovník" in Czech)

b) put the directive in a JS comment (in order to have the JS code valid for editors):
// +<import resource="...">

Anyone interested. Is it worth to file a separate "contribution bug"?

Comment by Kevin Roast [X] (Inactive) [ 27-Nov-14 ]

To be clear, this issue has not been ignored. It has been considered more than once.

It is a pervasive change that affects 100's of files in Alfresco (more since the change was submitted) and will affect 1000's of customers who already use the import feature in their custom webscripts. It breaks the existing implementation. It will require all customisations using imports to be rewritten. Therefore it is not a simple feature to pull in to the core.

We evaluate contribution based on usefulness, impact to the core, impact to our customers, tests provided and so on. There is no doubt of the usefulness of this change, but we need to evaluate the impact on when/if breaking changes are made. Sorry if it seems like we are ignoring issues, but I can tell you they are all looked at internally by the small number of engineers we have available in each area.

Comment by Andreas Steffan [ 27-Nov-14 ]

I don't think is has to be a introduced as a breaking change. Why not keep the old (invalid) legacy syntax available and at the same time provide a valid one for new code?

It might already be valuable to give people the chance to write syntactically correct "imports". That could at the very least stop tooling to complain/break on new code making use of imports.

Comment by Petr Bodnár [ 27-Nov-14 ]

Agreed with Andreas. When you change just the RhinoScriptProcessor and maybe maximally a couple of related classes , while making this change backwards compatible, then I don't see how this can affect "hundreds of files in Alfresco"...

As I have written before I can try to prepare a patch for the new version of Alfresco, based on the older version, it just seems strange to me that the Alfresco engineers are not capable of doing this on their own... (really please see my comment above, is it really that hard to implement now?)

Comment by David Webster [X] (Inactive) [ 28-Nov-14 ]

I had a chat with Kev about this yesterday, and will implement his suggested backwards-compatible fix.

Comment by David Webster [X] (Inactive) [ 18-Dec-14 ]

I've fixed this according to Kev's suggestion (HBF r92569); it's now possible to precede the <import tag with "//" to allow the .js files to be parsed as valid JavaScript.

Comment by Kevin Roast [X] (Inactive) [ 18-Dec-14 ]

We will look into adding a new syntax (keeping the old one which can now be proceeded by a valid JavaScript comment) that can achieve the full goal to fix this properly in the future.

Comment by Michael Böckling [ 18-Dec-14 ]

Any chance we'll get ECMAScript 6 in Nashorn when its available? They've got a module system figured out already (see https://people.mozilla.org/~jorendorff/es6-draft.html#sec-imports).

Comment by Axel Faust [ 18-Dec-14 ]

@Michael: Regardless of what's in Nashorn when it's included in Alfresco, don't get your hopes up that it'll be a magic bullet. Any module concept that needs to cater to Alfresco-specific requirements will need to be a custom solution - otherwise Alfresco will have to deal with a lot of internals of Nashorn which I'm sure they are not keen on.

Comment by Michael Böckling [ 18-Dec-14 ]

Maybe, but we'll only know that when its released. My guess is they will have to support a mechanism to resolve string identifiers in the imports section to files and streams anyway, so I'd expect hooks for some form of file system or URL mapping glue code, like registering a resolver.

Comment by David Webster [X] (Inactive) [ 19-Dec-14 ]

Opening and assigning to UI Unassigned to allow for future work.

Comment by Torsten Werner [ 05-Nov-15 ]

I have tried the workaround that David Webster mentioned: add a comment // before <import...

But it does not work in Alfresco Enterprise 5.0.1. A script is no longer imported if commented.

Comment by Axel Faust [ 05-Nov-15 ]

Torsten Werner: Nothing is allowed to precede an import directive except whitespace characters. Even if // was allowed you'd run the risk of accidentally commenting the first line of the script being imported. I suffered this issue when I imported two scripts and the first ended with a commented line, resulting in the first line of the next script also being commented due to lack of a line break being added during merging.

Comment by Petr Bodnár [ 05-Nov-15 ]

@Axel, I'm sorry but what exact problem do you have? Anyway, the appropriate fix should have been committed already, to quote David Webster:

I've fixed this according to Kev's suggestion (HBF r92569); it's now possible to precede the <import tag with "//" to allow the .js files to be parsed as valid JavaScript.

BTW My original suggestion (see above) was to better precede the tag with "//+" for example so your argument of inadvertently importing a file which you don't want wouldn't have the right weight

Comment by Torsten Werner [ 05-Nov-15 ]

I cannot access the site the r92569 is linking to. What exactly was committed there? Is it part of Alfresco enterprise 5.0.1?

Comment by Petr Bodnár [ 05-Nov-15 ]

Yeah, good question, only god knows probably...

Comment by Mark Rogers [X] (Inactive) [ 05-Nov-15 ]

No it was not Enterprise, it was to a development branch, HEAD-BUG-FIX.

It was merged to HEAD last January. revision #94891

Comment by Axel Faust [ 05-Nov-15 ]

Ok - the confusion may be due to the fact that there are two distinct implementation of ScriptResourceHelper in Alfresco - one in Surf and one on Repository tier. The Surf one has been fixed and the change is in both 5.0.d and 5.0.1. The Repository tier one has not been changed / adapted.

Generated at Thu Dec 13 16:05:09 GMT 2018 using JIRA 7.6.3#76005-sha1:8a4e38d34af948780dbf52044e7aafb13a7cae58.