Description
Using liveview with the
<transpile>
flag in the tiapp set to true (or not in the tiapp as it defaults to true) currently throws the below error [link to source](
https://github.com/appcelerator/liveview/blob/713d920a19f9a6b9a444f3de53d92118ad1439f3/build/liveview.js#L427)
- This occurs because the liveview code is attempting to overwrite the global.L variable, globals are defined as readOnly [see here](
https://github.com/appcelerator/titanium_mobile/blob/5ce893c20d97b8d62b100d331fa40c88744df2f2/iphone/Classes/KrollContext.m#L1101) and as all code in now run in strict mode this is now an error (credit to TIMOB-18465 for helping me figure this one out)
There's two ways we can fix this
1. Remove kTiPropertyAttributeReadOnly on global properties
2. Stop making all JS core run in strict mode by default by adding
modules: false
to the object passed to preset-env, which will have a knock on effect to ES6 module support
My vote goes for 1, but again this highlights the fact the transpilation step *_is going to cause breaking changes for people_*
[ERROR] Script Error {
[ERROR] column = 12;
[ERROR] line = 427;
[ERROR] message = "Attempted to assign to readonly property.";
[ERROR] sourceURL = "file:///Users/eharris/Library/Developer/CoreSimulator/Devices/19A348C6-7F86-4096-AEDE-90BB291BC971/data/Containers/Bundle/Application/D00FE479-9C4B-4ECD-84B6-1C1790C822FE/ticreateapp.app/app.js";
[ERROR] }
Steps to reproduce
Enable transpilation by adding the <transpile>true</transpile>
flag to your tiapp.xml
Build an app using liveview to iOS
Actual
Error above is thrown
Expected
No error thrown, liveview should work as normal
cc [~cwilliams] [~hknoechel] If we take option 1 I think we should also fix this on all properties? Pier also filed TIMOB-18475 for String. properties, would probably be sensible to check what's globals are writable on all platforms and align for parity I should also note that I've done both options and they both work
So, I think there's still a way to "fix" liveview to hack the L function in a way that doesn't try to assign to the global "L" function: It appears to hijack require itself, and in that it can simply pass a reference to "L" that invokes it's own override (right here: https://github.com/appcelerator/liveview/blob/713d920a19f9a6b9a444f3de53d92118ad1439f3/build/liveview.js#L733). That does mean user code could insist on the original L function versus a possibly patched one:
I mean this is effectively how it is passing along it's hacked require function, so I don't see why we couldn't do the same for the L override. I guess this raises a couple larger points: - Yes, transpilation could still break user code if they're doing very funky things like this that relied on non-strict behavior (i.e. actually overriding read-only globals!). In non-strict code though, wouldn't this hijacking of L actually silently fail? If the global L function is read-only non-strict code typically would silently ignore bad behavior and strict throws errors. See https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Strict_mode - Do we want to open up some of our API to be non read-only? Clearly liveview has a use for overriding some builtin API members. But this is beginning to play with fire if we don't want some/all of our API to be read-only.
I think one of the problems here is that we're not consistent across platforms, this code works on both Windows and Android (in strict mode) who allow global.L to be overwritten. iOS is the only platform which doesn't and the docs don't define it as read-only either. Looking at other globals in other environments node doesn't seem to complain when overriding in strict mode (from my fairly limited testing). I feel that, while this is fixable in liveview, given that (to my knowledge) we wouldn't be able ship the fix alongside 7.1.0 and that iOS is in the minority of how it behaves, we should be removing the
kTiPropertyAttributeReadOnly
on the global properties.Thought about it and agree with you [~cwilliams], I think parts of the global namespace should be read-only while others shouldn't (Ti/Titanium should be read-only for sure). Seeing as that's a problem for another day I've put up a fix for liveview, https://github.com/appcelerator/liveview/pull/109, tested across iOS and Android just need to test Windows
Progressing to in review, have tested across Android, iOS and Windows and it works
This fix has now shipped in Studio 5.1.0 which was released yesterday. Please read the release blogpost for instructions on how to update, https://www.appcelerator.com/blog/2018/08/ga-release-appcelerator-studio-5-1-0/ As it's shipped I'm now closing this ticket