dmachi / dojox_application

Dojox Application Framework for mobile, tablets, and desktops
Other
27 stars 24 forks source link

The app build support was not updated to deal with the has-config support when it was added. #193

Open edchat opened 11 years ago

edchat commented 11 years ago

We should do like dojo standard build, we include every has branch except if the has flag has been set in has staticHasFeature section in which case we use that value to include only what corresponds to it.

edchat commented 11 years ago

I was hoping to be able to take advantage of the staticHasFeatures in the build profile to set the "has" values to be used by DiscoverAppBuild and configProcessHas(config), but I ran into a few problems with this: 1) The staticHasFeatures are not setup with has.add() during the build, so the checks in configProcessHas(config) were still not working. I was able to get around this problem by calling has.add for each item in staticHasFeatures in DiscoverAppBuild before calling configProcessHas(config). But when I made this change I ran into the next problem (2). 2) When a staticHasFeatures is set for a "has" check, the build will update the code which checks that sets and uses "has" to always pass (or fail) based upon the staticHasFeature. This caused a problem when running from the built version because the config.json file was not updated to force those "has" checks to pass.
What ended up happening is this, when I tried to set "html5history": 1, in staticHasFeatures for the ContactsApp, (which could be seen in contactsApp.js.uncompressed.js) was that the call to has.add() which originally was like this: has.add("html5history", !has("ie") || has("ie") > 9); was replaced with this: 1 || has.add("html5history", !has("ie") || has("ie") > 9); So the has.add("html5history" was never called when running from the build, so when running from the build HistoryHash.js would be used instead of History.js

I see two possible solutions: 1) I could get around this problem by adding support for appHasFeatures to be used for things in the config instead of staticHasFeatures. So DiscoverAppBuild would call has.add for everything in appHasFeatures or staticHasFeatures, but things used in the config could only be set in appHasFeatures and not in staticHasFeatures. 2) Or I could try to have the build rewrite the config file to remove the has checks for things which are set in the staticHasFeatures.

Option 1 is easy to do, I am not sure how difficult it would be to do option 2.

dmachi commented 11 years ago

I'll have to defer to you on this. You have much more practical knowledge of the details here. The only other suggestion I can make is to ping Rawld and see if he has any other clever way to accomplish it.

Dustin

On Jul 25, 2013, at 12:33 PM, Ed Chatelain wrote:

I was hoping to be able to take advantage of the staticHasFeatures in the build profile to set the "has" values to be used by DiscoverAppBuild and configProcessHas(config), but I ran into a few problems with this: 1) The staticHasFeatures are not setup with has.add() during the build, so the checks in configProcessHas(config) were still not working. I was able to get around this problem by calling has.add for each item in staticHasFeatures in DiscoverAppBuild before calling configProcessHas(config). But when I made this change I ran into the next problem (2). 2) When a staticHasFeatures is set for a "has" check, the build will update the code which checks that sets and uses "has" to always pass (or fail) based upon the staticHasFeature. This caused a problem when running from the built version because the config.json file was not updated to force those "has" checks to pass.

What ended up happening is this, when I tried to set "html5history": 1, in staticHasFeatures for the ContactsApp, (which could be seen in contactsApp.js.uncompressed.js) was that the call to has.add() which originally was like this: has.add("html5history", !has("ie") || has("ie") > 9); was replaced with this: 1 || has.add("html5history", !has("ie") || has("ie") > 9); So the has.add("html5history" was never called when running from the build, so when running from the build HistoryHash.js would be used instead of History.js

I see two possible solutions: 1) I could get around this problem by adding support for appHasFeatures to be used for things in the config instead of staticHasFeatures. So DiscoverAppBuild would call has.add for everything in appHasFeatures or staticHasFeatures, but things used in the config could only be set in appHasFeatures and not in staticHasFeatures. 2) Or I could try to have the build rewrite the config file to remove the has checks for things which are set in the staticHasFeatures.

Option 1 is easy to do, I am not sure how difficult it would be to do option 2.

— Reply to this email directly or view it on GitHub.

edchat commented 11 years ago

I was able to do option 2 above.
I was able to add a custom transform for the app config which will process the config using only the staticHasFeatures, and will write out the updated config, so that the built code (which removes the has checks for things in the staticHasFeatures) will work with the built config which also removed the has checks for things in the staticHasFeatures. This seems like the right way to go.

dmachi commented 11 years ago

I agree, good job :)

Dustin

On Jul 30, 2013, at 4:12 PM, Ed Chatelain wrote:

I was able to do option 2 above.

I was able to add a custom transform for the app config which will process the config using only the staticHasFeatures, and will write out the updated config, so that the built code (which removes the has checks for things in the staticHasFeatures) will work with the built config which also removed the has checks for things in the staticHasFeatures. This seems like the right way to go.

— Reply to this email directly or view it on GitHub.