Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Show warning for deprecated short imports/require used in application #4375

Closed
rosen-vladimirov opened this issue Feb 19, 2019 · 14 comments
Closed
Assignees
Labels
Milestone

Comments

@rosen-vladimirov
Copy link
Contributor

Is your feature request related to a problem? Please describe.
In NativeScript 5.2.0 short imports like import * as application from "application"; are deprecated, i.e. support for them will be removed in a future version. It would be great in case CLI can analyze the application and report for such imports used in the app.
NOTE: Same is applicable for const application = require("application");

Describe the solution you'd like
During project build/run, CLI should analyze application's code and inform for used short imports. Same is applicable when running tns doctor inside project directory.

Describe alternatives you've considered
TSLint rule can do the same for TypeScript projects, but it will not work for JS ones. Also the IDE should have integration with TSLint to see the warning.

@rosen-vladimirov
Copy link
Contributor Author

NOTE: Until now, we were able to use short imports/requires for all packages coming from tns-core-modules, i.e. all directories from here: https://github.com/NativeScript/NativeScript/tree/master/tns-core-modules

This means, that we were able to write:

const fileSystem = require("file-system"); // In JS Project
import * as fileSystem from "file-system"; // In TS Project

From now on, in case you have such short imports/require, during tns doctor, tns prepare <platform>, tns run <platform>, tns build <platform> , tns debug <platform> and tns test <platform>, CLI will notify you that these short imports are deprecated and you should replace them in your code.

CLI will not notify you for short imports inside node_modules directory.

@tdhman
Copy link

tdhman commented Mar 12, 2019

Hello,
After upgrading to NS 5.2.0, I got a lot of warning because of short import in my project's assets. Since my application will need to execute some static javascript resources in webview, I have added these js as static assets under the folder app/assets, but when build the app, the cli show warning reports for also my js files. Is there any way to turn off the verification for resource files ?
Thanks.

@rosen-vladimirov
Copy link
Contributor Author

Hey @tdhman ,
Can you please post an example of some warnings generated for your application, so we can investigate them?

@tdhman
Copy link

tdhman commented Mar 12, 2019

Hello,
This is one of the import warning I got, actually I got this for every js file in my assets folder.

In file /../app/assets/www/IM.js line <This is content of my minified js>: is short import. Add tns-core-modules/ in front of the required/imported module.

@rosen-vladimirov
Copy link
Contributor Author

Hey @tdhman ,
Is it possible to send us the minified js file, so we can investigate the issue further. We have an idea what's going wrong, but it will be great in case you can send us the real world scenario for this case.
In case you do not want to send it here, you can find me in the NativeScript Community Slack - search for vladimirov

@miroslavaivanova
Copy link
Contributor

@tdhman You can try the fix with the rc version. Just install CLI like this npm i -g nativescript@rc.

@tdhman
Copy link

tdhman commented Mar 19, 2019

@miroslavaivanova I install the rc version as you suggested (+ nativescript@5.2.4-2019-03-19-13041) but it does not resolve the issue.

@rosen-vladimirov
Copy link
Contributor Author

Hey @tdhman ,
Can you please send us at least one of the minified files that triggers the issue you've faced. Without it we are unable to provide additional fix, as currently we are not sure what causes the incorrect behavior.
I'm looking forward to hearing from you.

@tdhman
Copy link

tdhman commented Mar 20, 2019

Hello @rosen-vladimirov, one of the minified js that cause the tns warning of import is UWA_Standalone_Alone.js. I added this file in my assets folder & when run tns build command, it shows me the warning

In file /../assets/www/UWA/js/UWA_Standalone_Alone.js line define("UWA/Embedded",["UWA/Core","UWA/String","UWA/Class","UWA/Class/Options","UWA/Class/Debug","UWA/Class/Events","UWA/Utils","UWA/Utils/InterCom"],function(e,t,i,n,o,s,r,a){"use strict";function d(e,t){var i;for(i in t)t.hasOwnProperty(i)&&(e.style[i]=t[i])}function l(e,t){var i,n;for(n in t)t.hasOwnProperty(n)&&(i=t[n],"styles"===n?d(e,i):"html"===n?e.innerHTML=i:"text"===n?(e.innerHTML="",e.appendChild(document.createTextNode(i))):e.setAttribute(n,i))}function h(e,t,i){var n=document.createElement(e);return t&&l(n,t),i&&i.appendChild(n),n}function c(e){var t;if("object"!=typeof e)return!1;for(t in e)if(e.hasOwnProperty(t))return!1;return!0}function u(e){return"boolean"==typeof e?e===!1?"0":"1":r.encodeUrl(e)}var p=i.extend(n,o,s,{url:"",id:null,data:null,version:e.version,elements:null,preferences:null,disableRemote:!0,defaultOptions:{container:null,className:"module",title:null,themeUrl:null,color:"white",height:"auto",width:"100%",maxHeight:!1,bookmarklet:!1,lang:null,id:null,readOnly:!1,data:{},buildHeader:!1,displayHeader:!0,displayFooter:!0,displayScroller:!1,displayEdit:!0,cache:null,useAsyncFrame:!1,useAppCache:!1,offlineMode:!1,autoLaunch:!0,subDomain:!1,subDomainPattern:"{id}.widget.",remoteName:"uwa.embedded",sandbox:!1,exposition:e.hosts.exposition,uwa:e.hosts.uwa},socket:null,init:function(e,t){this.url=e,this.data={},this.elements={},this.preferences=[],this._remoteQueue=[],this.startTime=(new Date).getTime(),this.setOptions(t),t=this.options;var i,n,o,s=typeof t.container;if("string"===s?this.container=document.getElementById(t.container):"object"===s&&null!==this.options.container?this.container=t.container:(o=document.getElementById("UWA_ASYNC"),this.container=h("div",{id:this.id,class:t.className}),o?o.parentNode.insertBefore(this.container,o.nextSibling):(i=document.getElementsByTagName("script"),n=i[i.length-1],n.parentNode.insertBefore(this.container,n))),p.Instances[this.id]=this,!this.container)throw new Error("UWA.Embedded is unable to get container element with container "+this.container+".");if("string"!=typeof this.url)throw new Error("UWA.Embedded expect url defined has first param");this.log('Init new instance with container id "'+this.id+'" and widget url "'+this.url+'"'),this.render()},destroy:function(){},setOptions:function(e){return this._parent(e),e=this.options,e.title&&(e.buildHeader=!0),e.buildHeader&&(e.displayHeader=!1),this.data=e.data||{},!e.id&&this.id||(this.id=e.id||this.generateId()),this},generateId:function(){var e;do e="uwa-"+r.getCheckSum(this.url)+"-"+r.random(1,1e5);while(document.getElementById(e)&&null!==p.Instances[e]);return e},render:function(){var t,i={},n=this.options,o=this.container,s=navigator.userAgent.toLowerCase().match(/ip(?:ad|od|hone)/);i.wrapper=h("div",{class:"moduleWrapper"}),n.buildHeader&&(i.header=h("div",{class:"moduleHeader",styles:{position:"relative",overflow:"hidden",height:"23px",padding:"5px 5px 0 5px"}},i.wrapper),i.iconContainer=h("span",{styles:{padding:"0 5px 0 0"}},i.header),i.icon=h("img",{src:n.uwa+e.paths.img+"icon.png",class:"moduleIcon",height:16,width:16,styles:{border:"none",height:16,width:16}},i.iconContainer),i.title=h("span",{text:n.title||"Loading...",class:"moduleTitle",styles:{font:'bold 11px/20px "Lucida Grande",Tahoma,Helvetica,Arial,sans-serif',verticalAlign:"top"}},i.header)),i.body=h("div",{class:"moduleContent"},i.wrapper),t={id:"frame-"+this.id,frameBorder:0,width:"100%",scrolling:n.maxHeight||"auto"!==n.height?"auto":"no"},n.sandbox&&(t.sandbox="allow-same-origin allow-forms allow-scripts allow-popups"),i.iframe=h("iframe",t,i.body),d(i.iframe,{display:"block"}),n.bookmarklet&&(n.displayScroller=!0,n.height="100%",n.width="320px",d(i.wrapper,{position:"fixed",top:0,right:0,height:n.height,width:n.width,"z-index":99999998})),"auto"!==n.height&&(i.iframe.height=n.height),s&&n.displayScroller&&(i.iframe.scrolling="no"),this.elements=i,o.innerHTML="",o.appendChild(i.wrapper),n.useAsyncFrame?this.renderIframeAsync():this.renderIframe(),this.setChromeColor(n.color),this.initRemote()},renderIframe:function(){var e=this.elements.iframe,t=this.getIframeUrl();e.previousIframeUrl&&e.previousIframeUrl===t||(e.previousIframeUrl=t,e.src=t)},renderIframeAsync:function(){var t,i=this,n=JSON.stringify,o=i.options,s=i.elements.iframe.contentWindow.document,r=o.exposition+"/widget/amd/js?uwaUrl="+u(i.url),a={id:i.id,widgetDomain:window.location.toString().replace(/#.*$/,"")};o.cache&&(r+="&cache="+u(o.cache)),t=["<!DOCTYPE html>","<html>","<head>",'<meta charset="UTF-8" />','<script type="text/javascript">',"   var UWA = {hosts:"+n(e.hosts)+"},",'       curl = {apiName: "require"};',"</script>",'<script type="text/javascript" src="'+o.uwa+"/lib/vendors/curl.js?v="+i.version+'"></script>','<script type="text/javascript" src="'+r+"&v="+i.version+'"></script>',"</head>","<body>",'<script type="text/javascript">','   define("'+i.id+'", ["'+i.url+'"], function (widget) {',"       UWA.extend(widget, "+n(a)+");","   });","</script>","</body>","</html>"],s.open(),s.write(t.join("\n")),s.close()},getIframeUrl:function(){var e,t,i=this,n=[],o=i.options,s=i.data,r={uwaUrl:i.url,id:i.id,cache:o.cache,header:o.displayHeader,footer:o.displayFooter,scroller:o.displayScroller,displayEdit:o.displayEdit,autoLaunch:o.autoLaunch&&!o.offlineMode,useAppCache:o.useAppCache,offlineMode:o.offlineMode,chromeColor:o.color,widgetDomain:window.location.toString().replace(/#.*$/,""),themeUrl:o.themeUrl,lang:o.lang};if(!o.offlineMode){r.readOnly=o.readOnly;for(e in s)s.hasOwnProperty(e)&&!r.hasOwnProperty(e)&&(r[e]=s[e])}for(e in r)r.hasOwnProperty(e)&&null!==r[e]&&n.push(e+"="+u(r[e]));if(t=i.getIframeDomain()+"?"+n.join("&"),t.length>2048)throw new Error("UWA.Embedded iframe url is more than 2048 characters please implement UWA.Embedded using offlineMode=true.");return t},getIframeDomain:function(){var e,t=this.options,i=t.exposition;return t.subDomain&&(e=r.getCheckSum(this.url),i=i.replace(": is short import. Add tns-core-modules/ in front of the required/imported module.

Sorry for my late response & thank you for your quick response on this issue.

@rosen-vladimirov
Copy link
Contributor Author

Hey @tdhman , thanks a lot, we've identified the issue and we've provided a fix in our next version.
Can you please use npm install -g nativescript@next and after that try tns run <platform> again, the warnings should be gone.

@tdhman
Copy link

tdhman commented Mar 21, 2019

Hello @rosen-vladimirov,

I tried the version nativescript@next and yes, it does not show any wrong short import warnings.
However, I cannot build & run my app with tns run <platform> command. It always shows me the error below. I have to rollback to nativescript@5.1.1 & everything works fine. I'm using XCode 10.1.

** ARCHIVE FAILED **

Unable to apply changes on device: <deviceId>. Error is: Command xcodebuild failed with exit code 65.

@rosen-vladimirov
Copy link
Contributor Author

Hey @tdhman
We are working on a fix for the xcodebuild error in our next version. Thanks for testing it!

@rosen-vladimirov
Copy link
Contributor Author

Hey @tdhman ,
We've fixed the issue, can you please try with latest next or rc version of CLI?

@tdhman
Copy link

tdhman commented Mar 22, 2019

@rosen-vladimirov I install the latest next version & the CLI works as expected.
Thanks a lot for your support.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants