[ALOY-1523] Alloy uses old require-paths with new require-behavior
GitHub Issue | n/a |
---|---|
Type | Bug |
Priority | Medium |
Status | Closed |
Resolution | Fixed |
Resolution Date | 2017-04-10T16:31:18.000+0000 |
Affected Version/s | n/a |
Fix Version/s | Release 6.2.0, alloy 1.9.11 |
Components | Runtime |
Labels | n/a |
Reporter | Hans Knöchel |
Assignee | Feon Sua Xin Miao |
Created | 2016-10-18T18:13:37.000+0000 |
Updated | 2017-06-14T20:56:42.000+0000 |
Description
We introduced an updated require-handling in Titanium 6 that matches the full NodeJS specs for handling all kind of files and data structures. Although we are save for older require-paths, we should update the Alloy-internal require-calls as well. Example log:
require called with un-prefixed module id: alloy/constants, should be a core or CommonJS module. Falling back to old Ti behavior and assuming it's an absolute path: /alloy/constants
It will only be shown in the DEBUG log-level, but adjusting it internally would be the cleanest way here. Thx!
PR: https://github.com/appcelerator/alloy/pull/801 *To Test* 1. Select SDK 6.X,
appc new -t titanium
2.DEBUG=* appc run -l trace
3. There should be norequire called with un-prefixed module id
in the output log 4. The app should run with no error 5. Updatetiapp.xml
to use sdk 5.X, the app should run with no errorThe PR looks fine (guess you did a regex/find-replace change here?). But as [~cwilliams] removed the debug log for 6.0.0 and it will break backwards compatiblity, we would need to bump Alloy for this as well. I did not think of that before, so we either bump Android to 2.0.0 in 6.0.0 (which is a) a bit too late now and b) not really worth for only this change; people would expect more in a 2.0.0) or we move this to a later Alloy / SDK version. I would suggest the latter for now, especially since there won't be any reports regarding those logs anymore and it does not break any current behavior. Thx!
[~hansknoechel], maybe I have missed something, what's the test case you used that shows the changes break backward compatibility? I tested with *6.0.0.v20161013072802* and *5.5.0.GA*, the app launches fine.
[~fmiao] See the comment from Chris in https://github.com/appcelerator/titanium_mobile/pull/8553.
[~hansknoechel], the paths are prefixed with a
/
, they are resolved relative to theResources
directory. This works in SDK 6.0.0 prior. It should also work in 6.X. https://github.com/appcelerator/titanium_mobile/blob/master/android/runtime/common/src/js/module.js#L262. That said, I do agree that this PR is not a deal breaker.Putting out of review for now, since Alloy 2.0.0 has not been branched if I see it correctly.
This is not a breaking change, why should it go into Alloy 2.0.0?
Is was marked for 2.0.0 :-) Happy to accept it earlier then. Probably I even set the version earlier. So would you suggest to put in in the next Alloy version / 6.1.0? You're the Alloy boss :-)
[~hansknoechel], I was curious, because you marked it for 2.0.0. ;) I assume it does break something that I overlooked.
Passed FR with: NPM Version: 3.10.10 Node Version: 6.10.1 Mac OS: 10.12.3 Appc CLI: 6.2.0 Appc CLI NPM: 4.2.9-3 iOS Device: 10 SDK 6.0.3.GA and 5.5.1 Tested with the above environment. "require called with un-prefixed module id" was not seen as it should not have. App works normally in Sim and on Device using SDK 6.0.3 and SDK 5.5.1.
Update fix version.
WPATH generation is still wrong. :( for example Alloy 1.10.2
[~darknos], created ALOY-1569 to track the issue, thanks for catching this!