[ALOY-1540] npm node_modules are erroring when compiling
GitHub Issue | n/a |
---|---|
Type | Bug |
Priority | Critical |
Status | Reopened |
Resolution | Unresolved |
Affected Version/s | n/a |
Fix Version/s | alloy 1.9.11 |
Components | Tooling |
Labels | n/a |
Reporter | Creative |
Assignee | Feon Sua Xin Miao |
Created | 2017-01-31T14:07:53.000+0000 |
Updated | 2019-09-05T15:59:17.000+0000 |
Description
I believe it was said that Ti SDK 6.0.0+ came with full node support. This should include installation of npm modules as well. Sadly, a lot of modules are not passing the compilation process of titanium, because they do not follow a certain convention.
One of those error messages would be:
[ERROR] Error generating AST for "/Users/john/Documents/Appcelerator_Studio_Workspace/myapp/Resources/iphone/node_modules/glob/test/00-setup.js"
[ERROR] 'return' outside of function
[ERROR] line 82, position 1779
[ERROR] Alloy compiler failed
Testcase:
Create a package.json (regular npm setup) inside app/lib so the packages come available during runtime. Install for instance mocha
(https://www.npmjs.com/package/mocha). There are many more packages that will fail the compilation process once installed into /app.
app/lib/package.json
{
"name": "myapp",
"description": "myapp",
"private": true,
"dependencies": {
},
"devDependencies": {
"mocha": "2.5.3",
"should": "7.1.1",
"ti-mocha": "0.2.0"
}
}
run npm install
in this directory to install the module.
I recommend installing a lot of packages, and determine if they pass the Alloy compiler. Every compilation should pass.
[~hansknoechel], Alloy compile uses Uglifyjs to create ast of js files. For the reported error,
'return' outside of function
, it looks like an issue with Uglifyjs, which tries to be spec-compliant and does not like global scope returns. There could be other issues with uglify a commonJS module. I'm looking into this. [~cwilliams], it'll also be an issue with SDK uglifying npm modules files under/Resources
dir TIMOB-24373.PR: https://github.com/appcelerator/alloy/pull/814
The version of uglify-js that Alloy uses is a bit out-of-date. I'd be surprised if the new version tolerated
return
outside of functions. We had talks of changing the build process so that any Uglify exceptions would just be warnings and not stop the build, however in Alloy's case, I'm pretty sure it needs a valid AST to generate the app.Alloy compile needs valid AST of controller to generate the app, it doesn't need to parse AST of the files in
app/lib
, they are copied toResources
. The question is does Alloy need to optimize all files underapp/lib
orapp/lib/node_modules
? Alloy can skip those files then we need to take care of TIMOB-24373.[~uzbbert] That's a great find, however we use UglifyJS, not UglifyJS2, and UglifyJS doesn't appear to have this option. :(
[~fmiao] Alloy should never optimize anything in a node_modules directory.
PR merged.
This is still an issue. Ti 8.1.1.GA, Alloy 1.14.0 1. create new project 2. create package.json in app/lib 3. in *app/lib* do *npm install cryptlib* (for example) 4. build the project Fails with
[ERROR] Alloy compiler failed
Doing a trace log shows: {{[DEBUG] if (!process.env.SAUCE_KEY || !process.env.SAUCE_USER) [DEBUG] return console.log('SAUCE_KEY and/or SAUCE_USER not set, not running sauce tests') [DEBUG] if (!/v0\.10/.test(process.version)) [DEBUG] return console.log('Not Node v0.10.x, not running sauce tests') [DEBUG] require('./sauce.js') [DEBUG] [DEBUG] /Users/jkneen/.nvm/versions/node/v9.9.0/lib/node_modules/alloy/Alloy/commands/compile/sourceMapper.js:212 [DEBUG] throw e; [DEBUG] ^ [DEBUG] SyntaxError: 'return' outside of function (4:2)}}I've created a PR that fixes this issue. If this is accepted as a feature for Alloy, I will create PR against Alloy master. https://github.com/brentonhouse/titanium-turbo/pull/52
[~bhouse] I'm fine with always enabling
allowReturnOutsideFunction: true
. I get why it's a flag, but many, many projects explicitly enable this feature including Titanium: https://github.com/appcelerator/node-titanium-sdk/blob/master/lib/jsanalyze.js#L77-L83.I personally don't see a reason for a try/catch and I'd vote to have it removed from Titanium.