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.

Sign up for GitHub

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

Jump to bottom

PHP files not being rewritten #214

Open
yellowled opened this issue Jan 3, 2017 · 10 comments
Open

PHP files not being rewritten #214

yellowled opened this issue Jan 3, 2017 · 10 comments

Comments

@yellowled
Copy link

I have a project in which a PHP file is no longer being rewritten by the grunt task since updating from 1.4.1 to 1.5.0. Hashing the (CSS/JS) assets works fine, but the references to those assets are no longer updated properly in the file.

According to my tests, it does not matter that it's a .php file or that the file name contains an underscore (or other special characters). It seems to have something to do with the actual content of the .php files, but I can't figure out what it is.

I use node 4.7.0 and grunt 1.0.1 on Mac OS 10.12.2.
@benhoIIand
Copy link
Owner

Can you create a basic test case for this in a PR and I can go about fixing the issue

@yellowled
Copy link
Author

I'm not sure I know what you mean by that and how to do it. (I'm not exactly a JS expert.)

However, I seem to have narrowed it down quite a bit. The CMS used in this particular project uses a PHP statement to emit the correct path to the assets directory. The reduced test case for this would be

<!DOCTYPE html>
<html>
<head>
    <link rel="stylesheet" href="<?php echo $config->urls->templates?>css/master.css">
</head>
<body>
    <script src="<?php echo $config->urls->templates?>js/master.js"></script>
</body>
</html>

If I remove the <?php … ?> bits, it works; so it's probably something about matching the angle brackets, I guess?

If that does not help, I'd need clear instructions as to what you from me need to be able to test this.

@benhoIIand
Copy link
Owner

Yes, it would appear that 1.5.0 has broken the ability to pickup assets that have alternate syntax inside of them, i.e. your example above. I would recommend hardcoding your grunt-cache-bust version to 1.4.1 so it still works for you.

@yellowled
Copy link
Author

Yeah, already pinned it to 1.4.1. Thanks.

@benhoIIand benhoIIand mentioned this issue Jan 5, 2017
Merged
@richardhinkamp
Copy link
Contributor

Hmm since we do not known what <?php echo $config->urls->templates?> will be, we can never determine if css/master.css is the file we bust, or a file in some other directory. This worked with lots of side effects. I don't know if it can be fixed without re-introducing those side effects. Maybe we need an option for strict file matching (what 1.5 does), which can identify relative paths etc or loose file matching (what 1.4 does). Or maybe specify <?php echo $config->urls->templates?> as a root "dir". Any thoughts @HollandBen ?

I considered this case broken, but when people are using it...

@benhoIIand
Copy link
Owner

benhoIIand commented Jan 5, 2017 •
edited
Loading

There were implementations before v1 which would replace terms like this to a base directory so it could be parsed correctly. It was removed in favour of a simpler "this is the absolute path of a file and replace all references with the new one". I don't see a way around this to be honest without bringing back some aspects of v0.x.x which were removed for a reason.

Saying that, one way I thought of was to have a loose option which, when set, would perform the "simpler find and replace" action that was there prior to #211 being merged.

@richardhinkamp
Copy link
Contributor

richardhinkamp commented Jan 5, 2017 •
edited
Loading

How about an option filePrefix which can be set to "<?php echo $config->urls->templates?>" so it can recognize the correct path, including relative paths etc? I have this working so that's nice, but it's not backward compatible.

If 1.5 breaks existing installs, it might have been better to use version 2.0.0 to signal that break?

@carlotrimarchi
Copy link

I updated today to the 1.5 version and I'm experiencing this issue as well, besides others.
Even worse, I'm trying with a simple url like this:

<link type="text/css" rel="stylesheet" href="/public/assets/css/style.css" media="screen">

And it manages to create the file with the hash, but doesn't update the reference anyway.

@richardhinkamp
Copy link
Contributor

@carlotrimarchi I'm very low on time to create fixes, but I added some test cases which failed and fixed those. Could you provide a test case for the simple url you mention? I want to fix normal path issues first, then see if we can find a solution to stuff like php tags in urls.

@PhilipsEkuma
Copy link 

<?php wp_head(); ?>

and

<?php wp_footer(); ?>

WordPress pulls in assets via the above tags in the header.php and footer.php files respectively. While the asset files are being rewritten without the

queryString: true

option, setting the

queryString: true

should append the busting strings to the referencing files but that does not happen. Even when the query string is set to false, the references in the php files are not updated either. What's to be done?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@carlotrimarchi @yellowled @benhoIIand @richardhinkamp @PhilipsEkuma