- Notifications
You must be signed in to change notification settings - Fork0
Danny-Engelman/CKTA
Folders and files
| Name | Name | Last commit message | Last commit date | |
|---|---|---|---|---|
Repository files navigation
These code examples come fromSPFx Examples submitted by MVPs
If you see your own code and feel ashamed. Good! You were writing oldskool ES3 code!
You mightfeel like Superman with TypeScript, but if you take off your cape,you are coding JavaScript ES6
And if you don't master ES6 you will still be writing oldskool ES3, long and error prone code
These are live examples where Clark Kent refactored Superman code
All code fragments are for educational purposes, to explain ES6 concepts, and are not necessarily production ready code
Links to Original code can be broken
A For loop and break to find something in an Array should be something of the past
privategetValueFromResults(key:string,results:ISearchResultValue[]): string{letvalue:string='';if(results!=null&&results.length>0&&key!=null){for(leti:number=0;i<results.length;i++){constresultItem:ISearchResultValue=results[i];if(resultItem.Key===key){value=resultItem.Value;break;}}}returnvalue;}
privategetValueFromResults(key:string,results:ISearchResultValue[]): string{letitem=results.find(result=>result.Key===key);returnitem ?item.Value :'empty string';}
And easily replace the too often used (or only one learned) .forEach method
.then((response:{PrimaryQueryResult:IPeopleDataResult}):void=>{letrelevantResults:any=response.PrimaryQueryResult.RelevantResults;letresultCount:number=relevantResults.TotalRows;letpeople=[];letpersona:IPersonaProps={};if(resultCount>0){relevantResults.Table.Rows.forEach(function(row){row.Cells.forEach(function(cell){//person[cell.Key] = cell.Value;if(cell.Key==='JobTitle')persona.secondaryText=cell.Value;if(cell.Key==='PictureURL')persona.imageUrl=cell.Value;if(cell.Key==='PreferredName')persona.primaryText=cell.Value;});people.push(persona);});}resolve(people);},(error:any):void=>{reject(this._peopleList=[]);}));
.then((response:{PrimaryQueryResult:IPeopleDataResult}):void=>{resolve(response.PrimaryQueryResult.RelevantResults.Table.Rows.map(row=>row.Cells.reduce((persona,cell)=>{if(cell.Key==='JobTitle')persona.secondaryText=cell.Value;if(cell.Key==='PictureURL')persona.imageUrl=cell.Value;if(cell.Key==='PreferredName')persona.primaryText=cell.Value;returnpersona;},{})// start with empty {} persona object));},(error:any):void=>reject(this._peopleList=[]));
Resolve and Returns statements, it is hard to see the flow of this functions
returnnewPromise((resolve,reject)=>{varwriteManifestsTask=undefined;for(vari=0;i<config.uniqueTasks.length;i++){if(config.uniqueTasks[i].name==='writemanifests'){writeManifestsTask=config.uniqueTasks[i];break;}}if(!writeManifestsTask){varerrorMsg='Couldn\'t retrieve the writeManifests task.';error(errorMsg);reject(errorMsg);return;}varurl=config.production ?`${writeManifestsTask.taskConfig.cdnBasePath}` :`${writeManifestsTask.taskConfig.debugBasePath}dist/`;varwebPartCodePath=`${config.libFolder}/webparts/angularMsGraph/AngularMsGraphWebPart.js`;varwebPartCode=fs.readFileSync(webPartCodePath,'utf-8');webPartCode=webPartCode.replace('$BASEURL$',url);fs.writeFile(webPartCodePath,webPartCode,(err)=>{if(err){error(err);reject(err);return;}log(`Base URL successfully set to${url}`);resolve();});});}
returnnewPromise((resolve,reject)=>{functiontaskerror(err){error(err);reject(err);}config.uniqueTasks.filter(task=>task.name==='writemanifests').forEach(task=>{leturl=config.production ?task.taskConfig.cdnBasePath :task.taskConfig.debugBasePath+"dist/";letwebPartCodePath=config.libFolder+"/webparts/angularMsGraph/AngularMsGraphWebPart.js";letwebPartCode=fs.readFileSync(webPartCodePath,'utf-8').replace('$BASEURL$',url);fs.writeFile(webPartCodePath,webPartCode,(err)=>{if(err)taskerror(err);log(`Base URL successfully set to${url}`);resolve();});});taskerror("Couldn't retrieve the writeManifests task.");}
Objective: Fade in (set opacity) N of M DOM elements
see source for all code, with some more refactoringit can be reduced to half the size
private_initializeOpacities():void{ leti=0;letj=1;letopacity;this.fadeIncrement=1/this.numCircles;for(i;i<this.numCircles;i++){letcircleObject=this.circleObjects[i];opacity=(this.fadeIncrement*j++);this._setOpacity(circleObject.element,opacity);}}private_setOpacity(element:HTMLElement,opacity: number):void{ element.style.opacity=opacity.toString();}
private_initializeOpacities():void{[...this.circleObjects].some((circle,idx)=>{circle.style.opacity=++idx/this.numCircles;returnidx===this.numCircles;//end .some() method when true});}
some() executes the callback function once for each element present in the array until it finds one where callback returns a truthy value (a value that becomes true when converted to a Boolean).If such an element is found, some() immediately returns true. Otherwise, some() returns false.
Same but shorter, without the return statement:
private_initializeOpacities():void{[...this.circleObjects].some((circle,idx)=>(circle.style.opacity=++idx/this.numCircles,idx===this.numCircles));}
Traditionaly we had to declare strings etc. up front before we could use them.
The one drawback is that your code no longer reads the same as it flows
With some careful refactoring you can not only save on used variables (and thus performance) but also shorten your code
privaterenderData():void{if(this.orders===null){return;//CK: Why does a void function return an undefined value?}letordersString:string="";//CK:String concation is oldskool since Microsoft (finally) implemented the .map /.reduce methods in IE9this.orders.forEach(order=>{ordersString+=`<tr> <tdpl-s1">${styles.number}">${order.id}</td> <tdpl-s1">${styles.number}">${newDate(order.orderDate).toLocaleDateString()}</td> <td>${order.region.toString()}</td> <td>${order.rep}</td> <td>${order.item}</td> <tdpl-s1">${styles.number}">${order.units}</td> <tdpl-s1">${styles.number}">$${order.unitCost.toFixed(2)}</td> <tdpl-s1">${styles.number}">$${order.total.toFixed(2)}</td></tr>`;});consttable:Element=this.domElement.querySelector(".data");table.removeAttribute("style");table.querySelector("tbody").innerHTML=ordersString;}
privaterenderData():void{if(this.orders){constclassName=styles.number;// GLOBAL!consttable:Element=this.domElement.querySelector(".data");table.removeAttribute("style");//could use standard HTML5 'hidden' attribute instead of whole Styletable.querySelector("tbody").innerHTML=this.orders.map(order=>`<tr> <tdpl-s1">${className}">${order.id}</td> <tdpl-s1">${className}">${newDate(order.orderDate).toLocaleDateString()}</td> <td>${order.region.toString()}</td> <td>${order.rep}</td> <td>${order.item}</td> <tdpl-s1">${className}">${order.units}</td> <tdpl-s1">${className}">$${order.unitCost.toFixed(2)}</td> <tdpl-s1">${className}">$${order.total.toFixed(2)}</td> </tr>`).join('');}}
publiccomponentWillReceiveProps(newProps:IPanelProps){if(newProps.isOpen===this.props.isOpen)return;clearTimeout(this._onCloseTimer);if(newProps.isOpen){if(!this.state.isOpen){this.setState({isOpen:true});}else{this.setState({isVisible:true});}}if(!newProps.isOpen&&this.state.isOpen){this._close();}}
Function returnsundefined if it is not the currect property-pane, alter the code to only execute the body
Negations!this.state.isOpen are hard to read, since there IS an else branch, the then/else can be reversed
The first condition of the last (3rd) IF is actually the else branch, so no need to check something twice
publiccomponentWillReceiveProps(newProps:IPanelProps){if(newProps.isOpen!==this.props.isOpen){clearTimeout(this._onCloseTimer);if(newProps.isOpen){if(this.state.isOpen)this.setState({isVisible:true});elsethis.setState({isOpen:true});}elseif(this.state.isOpen){this._close();}}}
Note: In the above example you can replace 'this' with a constant to make it minifiy better!
privatesetButtonsEventHandlers():void{constwebPart:JsomCrudWithBatchWebPart=this;this.domElement.querySelector('button.create-Button').addEventListener('click',()=>{webPart.createItem();});this.domElement.querySelector('button.readall-Button').addEventListener('click',()=>{webPart.readItems();});this.domElement.querySelector('button.update-Button').addEventListener('click',()=>{webPart.updateItem();});this.domElement.querySelector('button.delete-Button').addEventListener('click',()=>{webPart.deleteItem();});}
privatesetButtonsEventHandlers():void{constwebPart:JsomCrudWithBatchWebPart=this;letaddClick=(crud,func)=>webPart.domElement.querySelector(`button.${crud}-Button`).addEventListener('click',()=>func);addClick('create',webPart.createItem);addClick('readall',webPart.readItems);addClick('update',webPart.updateItem);addClick('delete',webPart.deleteItem);}
privateupdateItemsHtml(items:IListItem[]):void{constitemsHtml:string[]=[];for(leti:number=0;i<items.length;i){itemsHtml.push(`<li>${items[i].Title} (${items[i].Id})</li>`);}this.domElement.querySelector('.items').innerHTML=itemsHtml.join('');}
privateupdateItemsHtml(items:IListItem[]):void{this.domElement.querySelector('.items').innerHTML=items.map(item=>`<li>${items.Title} (${items.Id})</li>`).join('');}
varstringOfId='';data.forEach(element=>{stringOfId+=element.Id+',';// this will add a trailing ,});beforeCallback.updateStatus(`Item with IDs:${stringOfId} successfully updated`);
varstringOfId=data.map(element=>element.Id).join(',');beforeCallback.updateStatus(`Item with IDs:${stringOfId} successfully updated`);
Classic example where TypeScript strong Typing forces you to write duplicate code
Abstract endpoint calling into one method..
just think how many references you need to check when the HttpClient is upgraded to V2
publicgetLists():Promise<IList[]>{varhttpClientOptions :ISPHttpClientOptions={};httpClientOptions.headers={'Accept':'application/json;odata=nometadata','odata-version':''};returnnewPromise<IList[]>((resolve:(results:IList[])=>void,reject:(error:any)=>void):void=>{this.context.spHttpClient.get(this.context.pageContext.web.serverRelativeUrl+`/_api/web/lists?$select=id,title`,SPHttpClient.configurations.v1,httpClientOptions).then((response:SPHttpClientResponse):Promise<{value:IList[]}>=>{returnresponse.json();}).then((lists:{value:IList[]}):void=>{resolve(lists.value);},(error:any):void=>{reject(error);});});}publicgetList(listName: string):Promise<IListItem[]>{varhttpClientOptions :ISPHttpClientOptions={};httpClientOptions.headers={'Accept':'application/json;odata=nometadata','odata-version':''};returnnewPromise<IListItem[]>((resolve:(results:IListItem[])=>void,reject:(error:any)=>void):void=>{this.context.spHttpClient.get(this.context.pageContext.web.serverRelativeUrl+`/_api/web/lists('${listName}')/items?$select=Id,Title`,SPHttpClient.configurations.v1,httpClientOptions).then((response:SPHttpClientResponse):Promise<{value:IListItem[]}>=>{returnresponse.json();}).then((listItems:{value:IListItem[]}):void=>{resolve(listItems.value);},(error:any):void=>{reject(error);});});}
publicgetEndpoint(endpoint:string):Promise<IList[]>{varhttpClientOptions :ISPHttpClientOptions={};httpClientOptions.headers={'Accept':'application/json;odata=nometadata','odata-version':''};constcontext=this.context;returnnewPromise((resolve:(results:any)=>void,reject:(error:any)=>void):void=>{context.spHttpClient.get(context.pageContext.web.serverRelativeUrl+endpoint,SPHttpClient.configurations.v1,httpClientOptions).then(response=>response.json()).then(listdata=>resolve(listdata.value)).catch(error=>reject(error));})}publicgetLists():Promise<IList[]>{returngetEndpoint('/_api/web/lists?$select=id,title');}publicgetList(listName: string):Promise<IListItem[]>{returngetEndpoint(`/_api/web/lists('${listName}')/items?$select=Id,Title`);}
