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

Multiple class selectors on same element in a PHP file #141

Open
polygbrl opened this issue Sep 9, 2021 · 6 comments
Open

Multiple class selectors on same element in a PHP file #141

polygbrl opened this issue Sep 9, 2021 · 6 comments

Comments

@polygbrl
Copy link

polygbrl commented Sep 9, 2021

When using gulp-rcs ^3.0.0 and rcs-core ^3.6.5 with a PHP document including elements with multiple class selectors, these element's classlists are not transformed.

For example:

<?php #This is a php file ?>
<div class="hello fade-in"></div>
<div class="hello"></div>
<div class="fade-in"></div>

Transforms to:

<?php #This is a php file ?>
<div class="hello fade-in"></div>
<div class="t"></div>
<div class="n"></div>

If I change the PHP file to an HTML file and keep the same gulp-rcs config, everything goes as espected.

Same example transforms to:

<!--?php #This is a php file ?-->
<div class="t n"></div>
<div class="t"></div>
<div class="n"></div>

There doesn't seem to be anything related in the caveats list. Any hints on what is going on here?

@polygbrl
Copy link
Author

polygbrl commented Sep 9, 2021

As far as I understand it (which might be quite limited), rcs-core will consider PHP files as a string and therefore use the string regex to match any words between two " or two '. In this case multiple classes as "hello fade-in" will not match as white spaces excludes them from being considered as a class.

When using a HTML file, rcs-core uses parse5 to traverse the file and return an abstract syntax tree which in return allows it to map classes and replace them.

Then, parse5 can build an AST from some PHP as it considers all PHP code as #comment nodes. In this case, rcs-core could use it to traverse the document but we would then miss any HTML markup containing classes which is printed with PHP. Maybe that we could then run the regex on the PHP parts?

@JPeer264
Copy link
Owner

Hey. Sorry for the late response. You are right PHP is not yet supported and will be taken, as you already stated, as "any" instead of "html". However before we go any deeper into development, would you except to change variables within PHP as well? Something like:

<?php
$some_class = "some-class"; 

to transform to e.g.:

<?php
$some_class = "a"; 

Then, parse5 can build an AST from some PHP as it considers all PHP code as #comment nodes. In this case, rcs-core could use it to traverse the document but we would then miss any HTML markup containing classes which is printed with PHP. Maybe that we could then run the regex on the PHP parts?

parse5 can have a custom TreeAdapter, so I think this can be used to extend the functionality for php tags (need to look a little further into). If that does not work I think htmlparser2 seems to be a promising alternative to parse5 (with a little better community as well).

@polygbrl
Copy link
Author

No problems at all! In my opinion, it should at least fully work on the HTML parts of a PHP file but if this enhancement should lead rcs-core to officialy and fully support PHP then yes, I would expect it to change variables. This will lead to a lot of annoying cases, here are some examples I could think of:

<?php 
  $some = "some-class";
  $other = someAPICall() ? $some : "other-class";
  
  include 'classes.php';
?>

/* needs to deeply parse the above php */
<div class="<?php echo $other; ?>"></div>

/* needs to parse the above php and the inline html */
<div class="<?php echo $some; ?> default-class"></div>

/* breaks the attributes array on both parse5 and htmlparser2 because of the 
first opening " in php which is closing the html's class attribute declaration */
<div class="<?php echo "a-class", $bClass; ?>"></div>

 /* needs to teach rcs-core to look at classes.php after this? */
<div class="<?php echo $aClassFromIncludedFile; ?>"></div>

parse5's custom TreeAdapter looks great. I have to take a better look at htmlparser2 which seems to be a good and fast candidate too. After aiming at the <?php ?> parts do you intend to use something like php-parser to better go through above cases? By using it we could properly parse left and right hand side of any assign expressions for example.

I can't wrap my head around how to go through all this in a robust way, I probably need to understand better rcs-core's strategies but feel free to poke me if I can help in any way!

@JPeer264
Copy link
Owner

Ok so I think as first iteration it can be HTML inside PHP files only (I will work either later today, or on the weekend) and second iteration (if that works reliable) php-parser, as you suggested, can be taken into consideration as well. Latter might take some more research I guess.

@polygbrl
Copy link
Author

polygbrl commented Sep 16, 2021

Great! So for first iteration, using html replacer works as expected:

const php = rcs.replace.html(fs.readFileSync('./src/index.php', 'utf8'));

Or here with gulp-rcs:

html: ['.html', '.html', '.php'],

So I think simply running a regex like this on the output to replace the commented out PHP: <!--?php and ?--> with <?php and ?> would be enough?

/(<!--\?php).*(\?-->)/gm

But rcs-core should have a proper replacePhp as second iteration will probably involve a lot more than just this.
I'll be able to take a look at it this weekend too.

@pakpenyo
Copy link

Any news on this?

With Gulp 5:

Before
<ul class="column__4" aria-label="<?php bloginfo( 'name' ); ?>"></ul>

After
<ul class="fcolumnl" aria-label="<?php bloginfo( 'name' ); ?>"></ul>

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

No branches or pull requests

3 participants