Making some Yii vars protected instead of private

Qiang, I'm working on some additional functions to extend CClientScript but to tie in a bit better I need most of the variables to be protected instead of private.

Would you consider these to become protected instead of private?

This can be also true for functions, sometimes it can be better to declare these as protected instead of private perhaps.

I'm reluctant to change these variables to be protected mainly because they are tightly related with implementation and are very likely to be changed in future.

Could you describe why you need to access them directly?

I can understand that completely. 1.0.3 is now ready for merging and compressing scriptfiles. but currently the merging and compressing is not done there (yet).

To integrate JSMin (and later on CSSTidy) with minimal impact on already existing code I would like to read $_scriptFiles.

Was planning to do this by extending CClientScript and redefining some of the current functions.

After feeding them to JSMin, I can adjust the $_scriptFiles and $_coreScripts with $scriptMap. But will try it first without touching private vars and functions directly.

For normal scripts it's not that hard, but doing it for CoreScripts is a bit more difficult without additional tricks.

We actually don't plan to do merging and compressing.

I'm interested to know how you do merging and compressing with jsmin and csstidy. Could you please explain?

Currently I use the following code (JS only):



class ExtendedClientScript extends CClientScript


{


	public $options;





	public function registerCombinedScriptFiles($urls,$position=self::POS_HEAD)


	{


		if (!is_array($this->options))


		{


			$this->options['minify'] = YII_DEBUG ? false : true;


			$this->options['scriptPath'] = Yii::app()->assetManager->basePath;


			$this->options['scriptUrl'] = Yii::app()->assetManager->baseUrl;


			$this->options['basePath'] = Yii::app()->assetManager->basePath;


		}





		$mtimes = array();





		foreach ($urls as $asset)


			$mtimes[] = filemtime($this->options['basePath'].'/'.trim($asset,'/'));





		$combineHash = substr(md5(implode('',$urls)),0,10);


		$optionsHash = substr(md5(serialize($this->options)),0,10);


		$changesHash = substr(md5(serialize($mtimes)),0,10);





		$fileName = "$combineHash$optionsHash$changesHash.js";





		$renewJSFile = (file_exists($options['scriptPath'].'/'.$fileName)) ? false : true;





		if ($renewJSFile)


		{


			$this->removeObsolete('js', $combineHash, $optionsHash);





			foreach ($urls as $key=>$file)


				$combinedJS .= file_get_contents($this->options['basePath'].'/'.$file);





			if ($this->options['minify'])


				$combinedJS = JSMin::minify($combinedJS);





			file_put_contents($this->options['scriptPath'].'/'.$fileName, $combinedJS);


		}





		$this->registerScriptFile($this->options['scriptUrl'].'/'.$fileName, $position);


	}





	private function removeObsolete($type, $combineHash, $optionsHash)


	{


		$obsolete = glob($this->options['scriptPath']."/$combineHash$optionsHash*.$type");


		if (is_array($obsolete))


		{


			foreach ($obsolete as $remove)


				@unlink($remove);


		}


	}


}


It's nothing more than combining the scripts to one file and optionally compressing them.

JSMin is a static class, and I use it as a helper.

If I can read the $_scriptFiles, I can easily combine and compress the used core scripts as well.

Here a new version with JSMin and CssTidy. Cleaned some parts a bit. If there was an option to read the $_scriptFiles, I could extend the render function to do all this automatically, and point the $scriptMap to the generated file.

Currently I call the function in a filter in my BaseController class:



	public function filterMainSetup($filterChain)


	{


		$combineJS[] = '/js/ui/ui.core.js';


		$combineJS[] = '/js/ui/ui.tabs.js';


		$cs->registerCombinedFiles('js',$combineJS,$position=CClientScript::POS_HEAD);





		$combineCSS[] = '/css/layout.css';


		$combineCSS[] = '/css/main.css';


		$cs->registerCombinedFiles('css',$combineCSS);





		$filterChain->run();


	}


The class (needs jsmin in components and csstidy in extensions) :






class ExtendedClientScript extends CClientScript


{


	public $minify = true;


	public $scriptPath;


	public $scriptUrl;


	public $basePath;


	public $ttlDays = 1;


	public $prefix = 'c_';


	public $cssTidy = true;


	public $cssTidyTemplate = "highest_compression";


	public $cssTidyConfig = array(


				  'remove_bslash' => TRUE,


				  'compress_colors' => TRUE,


				  'compress_font-weight' => TRUE,


				  'lowercase_s' => FALSE,


				  'optimise_shorthands' => 1,


				  'remove_last_,' => TRUE,


				  'case_properties' => 1,


				  'sort_properties' => FALSE,


				  'sort_selectors' => FALSE,


				  'merge_selectors' => 2,


				  'discard_invalid_properties' => FALSE,


				  'css_level' => 'CSS2.1',


				  'preserve_css' => FALSE,


				  'timestamp' => FALSE


			 );





	public function registerCombinedFiles($type, $urls, $position=self::POS_HEAD)


	{


		if (YII_DEBUG) $this->minify = false;


		$this->scriptPath or $this->scriptPath = Yii::app()->assetManager->basePath;


		$this->scriptUrl or $this->scriptUrl = Yii::app()->assetManager->baseUrl;


		$this->basePath or $this->basePath = Yii::app()->assetManager->basePath;





		$mtimes = array();





		foreach ($urls as $asset)


			$mtimes[] = filemtime($this->basePath.'/'.trim($asset,'/'));





		$combineHash = substr(md5(implode('',$urls)),0,10);


		$changesHash = substr(md5(serialize($mtimes)),0,10);





		$optionsHash = ($type == 'js') ? substr(md5($this->basePath.$this->minify.$this->ttlDays.$this->prefix),0,10):


			substr(md5($this->basePath.$this->cssTidy.$this->ttlDays.$this->prefix.serialize($this->cssTidyConfig)),0,10);





		$fileName = "{$this->prefix}$combineHash$optionsHash$changesHash.$type";





		$renewFile = (file_exists($this->scriptPath.'/'.$fileName)) ? false : true;





		if ($renewFile)


		{


			$this->garbageCollect($type);





			foreach ($urls as $key => $file)


				$combinedFile .= file_get_contents($this->basePath.'/'.$file);





			if ($type == 'js' && $this->minify)


				$combinedFile = JSMin::minify($combinedFile);





			if ($type == 'css' && $this->cssTidy)


			{


				Yii::import('application.extensions.csstidy.*');


				require_once('class.csstidy.php');





				$cssTidy = new csstidy();


				$cssTidy->load_template($this->cssTidyTemplate);





				foreach($this->cssTidyConfig as $k => $v)


					$cssTidy->set_cfg($k, $v);





				$cssTidy->parse($combinedFile);


				$combinedFile = $cssTidy->print->plain();


			}





			file_put_contents($this->scriptPath.'/'.$fileName, $combinedFile);


		}





		($type == 'js') ? $this->registerScriptFile($this->scriptUrl.'/'.$fileName, $position):


			$this->registerCssFile($this->scriptUrl.'/'.$fileName);


	}





	private function garbageCollect($type)


	{


		$files = CFileHelper::findFiles($this->scriptPath, array('fileTypes' => array($type), 'level'=> 0));


		$ttl = $this->ttlDays * 60 * 60 * 24;





		foreach($files as $file)


		{


			if (strpos($file, $this->prefix) !== false)


			{


				if ((fileatime($file) + $ttl) < time() )


				{


					@unlink($file);


				}


			}


		}


	}


}


I know there can be some speedloss bcz of the filemtime routine, but I believe the lesser amount of data send to the client is more important. One could cache the filename to lessen the burden.

Your implementation looks good!

Have you thought about if you merge several CSS files that are under different directories, there might be problem with image files included in the CSS?

I'm still not sure whether or not to make those private members into protected…

Thanks, for css images I personally use relative to webroot paths, but relative paths can give an issue indeed. Perhaps better to put the output file in the publish directory.

Well if there was a getter for $_scriptFiles and $_cssFiles I can integrate without the need of protected vars, by using $scriptMap.

I just changed them to protected. Thanks.

Thanks, will be really helpful.

About $scriptMap: In the doc you state that:



$cs->scriptMap=array(


    'jquery.js'=>'/js/all.js',


    'jquery.ajaxqueue.js'=>'/js/all.js',


    'jquery.metadata.js'=>'/js/all.js',


    ......


);


Will include ‘/js/all.js’ once.

But according to my findings this is not the case, the paths are just substituted for the new path, and the script tag is generated as many times as originally (in this case 3x), but with the new path.

Using '*.js' works as described, but because of lack of position parameter will replace ALL scripts on ALL positions, and probably not really wanted.

Thank you! Just fixed this bug.

*.js still uses position information.

Cool. Have the code working, and ties in pretty good. Probably some others will find it useful. Since its not an extension (more like a component), were to put it?

A component IS an extension. ;)

You may share it in our extension system.