Is it considered bad practice to have PHP in your JavaScript

Question

So many times on this site I see people trying to do things like this :

<script type="text/javascript">
  $(document).ready(function(){

     $('<?php echo $divID ?>').click(funtion(){
       alert('do something');
     });

  });
</script>

I don't think that this is some sort of pattern that people naturally fall into. There must be some sort of tutorial or learning material out there that is showing this, otherwise we wouldn't see it so much. What I'm asking is, am I making too big a deal of this or is this a really bad practice?

EDIT : Was speaking to a friend of mine about this who often puts ruby in his JavaScript and he brought up this point.

Is it ok to dynamically place application wide constants in your JavaScript so you don't have to edit two files. for example...

MYAPP.constants = <php echo json_encode($constants) ?>;

also is it OK to directly encode data you plan to use in a library

ChartLibrary.datapoints = <php echo json_encode($chartData) ?>;   

or should we make an AJAX call every time?

Answer 1

Typically, it is bad practice to use language X to generate code in language Y.

Try decoupling the two languages by making data their only interface -- don't mingle the code.

In your example, you could improve the code by using PHP to populate a cfg structure that's available to JavaScript:

<script type="text/javascript">
  var cfg = {
    theId: "<?php echo $divID ?>",
    ...
  };

  $(document).ready(function(){
     $("#" + cfg.theId).click(funtion(){
       alert('do something');
     });
  });
</script>

This way, PHP only cares about populating the data structure and JavaScript only cares about consuming the data structure.

This decoupling also leads the way to loading the data asynchronously (JSON) in the future.

Update:

To answer the additional questions you asked with your update, yes, it would be good practice to apply the DRY principle and let PHP and JavaScript share the same configuration object:

<script type="text/javascript">
  var cfg = <?php echo json_encode($cfg) ?>;

  ...

There is no harm in inserting the JSON representation of your configuration directly into your page like this. You don't necessarily have to fetch it via XHR.

Answer 2

Dynamically generated JavaScript is a horrible, bad practice.

What you're supposed to do is comprehend what Seperation of Concerns and Progressive Enhancement mean.

This basically means you have dynamic HTML and static JavaScript (which enhances the HTML).

In your case you probably want a class on your div and select it with a class selector

Answer 3

The biggest problem with your snippet is you're missing the # to make it a valid jQuery selector ;).

I would say you should try avoid including PHP in your JavaScript where possible. What's wrong with changing the selector in your click() handler to a class, and adding the class to the element in question if you want the handler to be fired, and not if you don't;

<script type="text/javascript">
  $(document).ready(function(){

     $('.foo').click(funtion(){
       alert('do something');
     });

  });
</script> 

<div id="bar" class="<?php echo ($someCond ? 'foo' : ''); ?>">Hello</div>

There are circumstances where you need to include PHP in your JavaScript; but I must admit these are few and far between.

Once example is when you have different environments; test, staging and live. Each of them have a different location of your assets (images, mainly). The easiest way to set the path so that it can be used by JavaScript is something like;

var config = { assets: "<?php echo $yourConfig['asset_url']; ?>" };

Answer 4

This is a bad practice in my opinion, as you would need to call this file something.php and then you couldn't compress it for example, not mentioning it isn't ok to mix in your server stuff with your JavaScript. Try to limit the mixing between PHP and JS as much as possible.

You can always do this instead:

function setOnClick(divid) {
 $(divid).click(funtion(){
   alert('do something');
 });
}

And then you could call this function in a php file, to make these mixing things as small as possible.

$(function() {
  setOnClick('<?php echo $divId; ?>');
});

By doing this (having bigger JS files, not 2-3 lines where it doesn't matter) you can take advantage of compressing the JS file and front-end developers feel much comfortable working only with JavaScript in my opinion (as you could write Python, Ruby etc not only PHP - and the code could get bigger and bigger depending on what you need to do there).

Answer 5

I would consider this bad practice. When putting dynamic content inside script blocks you always must be aware of the fact that escaping inside a javascript context is not as simple as you would hope. If values were user-provided, it is not sufficient to html-escape them.

The OWASP XSS cheat sheet has more details, but basically you should adopt this pattern:

<script id="init_data" type="application/json">
    <?php echo htmlspecialchars(json_encode($yourdata)); ?>
</script>

Then in a separate .js file linked from your main html, load this code:

var dataElement = document.getElementById('init_data');
var jsonText = dataElement.textContent || dataElement.innerText  // unescapes the content of the span
var initData = JSON.parse(jsonText);

The reason for using a separate .js file is two-fold:

• It is cacheable so the performance is better
• The HTML parser is not triggered, so there is no risk of a XSS bug slipping through by someone putting a quick <?php tag in there somewhere

Answer 6

I don't think this is bad practice. If the ID required in your JavaScript is dynamic there is no other way to do this.

Answer 7

Some people would argue that it's bad practice. Not because it's PHP inside JS, but because it's inline JS and so won't be cached by the browser for ease of loading the next time.

IMO it's always better to use JSON to pass variables between the 2 languages, but I guess it's up to you.

Answer 8

The only thing I can think off that can really cause issues is when PHP errors are set to be displayed and so it shoves a load of HTML showing the PHP error into your JavaScript.

Also because its it in script it therefore doesn't show and it can sometimes take a while to realize why your script is broken.

Answer 9

I would say that in general don't do it. However if you want to pass data from PHP -> Javascript it would not strike me as crazy to have an inline Javascript block where you have code of the form shown bellow. Here the code is just passing data from php to javascript, not creating logic on the fly or the like. The good part of doing this vs an ajax call is that the data is available as soon as the page loads and does not require an extra trip to the server.

<script>
window.config = <?php echo json_encode($config);?>;
</script>

Of course an other option is to build a javascript config file from PHP via some form of build script that will put that into a .js file.

Answer 10

It depends by whom, and if you ask me, yes I do consider it back practice for a few reasons. First of all, I'd prefer to have javascript code in its own JS file that the php parser would not be able to touch.

Secondly, php executes at server time only, so if you are depending on some variable in php to change your javascript, that may not work very well. If there is some page-load setting you want to control with javascript, I typically prefer to add that value to the DOM with php so that javascript can reach it when and if it wants to (in a hidden div, for example).

Finally, just for organizational purposes, this can get very annoying. It's bad enough to mix html and php (in my opinion).

Answer 11

Containing the PHP into a config data object goes 90% of the way but best practice is to seperate it entirely. You can use a RESTful api to only request the data that you need, it is a bit more javascript but with a few advantages.

• Script is static and can be cached permanently
• PHP no longer an XSS Vector
• Complete seperation of concerns

Downsides:

• Requires an extra HTTP request
• more complex javascript

Script

//pure javascript
$.on('domready',function({
    //load the data
    $.get({
       url:'/charts/3D1A2E', 
       success: function(data){
           //now use the chart data here
           ChartModule.init(data);
       }
    });
})

Answer 12

It's not a bad practice ONLY if it's used for javascript code initialisation, (in my WordPress themes, I initialize my javascript objects with php functions like site_url() because it's the only way to handle that (maybe we could use an ajax request to get a json, and so... but it's a pain in the ass).

Good practice:

new javascriptObject("");

Bad practice:

/* some code*/ document.get_element_by_id(); /* some code*/