Skip to content

update phpserver to support versioned static urls #10250

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

Merged
merged 4 commits into from
Jul 18, 2017

Conversation

Flyingmana
Copy link
Member

@Flyingmana Flyingmana commented Jul 16, 2017

the phpserver implementation did not support versioned static urls before, which is solved by this change

I decided to just do a redirect, as it should be enough for dev users, as they usually use no-cache or hard reloads to flush any css files used anyway.

You can reproduce this issue by following the normal setup flow (install via cli) and generate the frontend files.
Then use the php builtin server as a webserver and then load the home page.
Result without this patch will be a bunch of 404 responses for css files.
After this patch, they can get resolved correctly.

@okorshenko okorshenko self-assigned this Jul 17, 2017
@okorshenko okorshenko added this to the July 2017 milestone Jul 17, 2017
@okorshenko
Copy link
Contributor

okorshenko commented Jul 17, 2017

Hi @Flyingmana
Could you please provide step-by-step instruction how we can reproduce the issue?
Thank you

Also, please fix the code. It throws a fatal error due to a syntax error. thank you

@Flyingmana
Copy link
Member Author

Iam really sorry, I did copy&paste it over different places before submitting it as a PR, seems one of them did replace the quotes...

How detailed do you need the step by step? And which step in the initial description was not detailed enough?

@ishakhsuvarov ishakhsuvarov self-assigned this Jul 18, 2017
@ishakhsuvarov
Copy link
Contributor

ishakhsuvarov commented Jul 18, 2017

Hi @Flyingmana
I've been trying to manually test according to steps you provided and did not get any success due to Too Many Redirects error all the time.

I've looked at the code and tried to improve the Regular Expression, fixing the coding style errors on the way. Now it works as expected for me. Please check if it works fine for you as well.
Thanks!

@Flyingmana
Copy link
Member Author

Will test it out in the evening, but the changes look ok for me.

Was the endless redirect related to a version string which contained something else then numbers?
Did the '/static' part of the replace make problems?

@ishakhsuvarov
Copy link
Contributor

@Flyingmana During my testing it did not really replace anything on each iteration, I've also ran some tests with the expression in question and didn't see it work as expected

@magento-team magento-team merged commit e42e09a into magento:develop Jul 18, 2017
magento-team pushed a commit that referenced this pull request Jul 18, 2017
[EngCom] Public Pull Requests

 - MAGETWO-70817: remove redundant else and use strict check #10271
 - MAGETWO-70787: update phpserver to support versioned static urls #10250
 - MAGETWO-70764: Fix overwrite default value image/file with NULL #10253
 - MAGETWO-70761: Show values that are duplicate in attribute error msg #10213
 - MAGETWO-70758: Fix for #4530 $product->getRatingSummary() always returns null […] #10248
 - MAGETWO-70706: simplify product lists #2 #9019
 - MAGETWO-70669: Fix fatal errors with deleteOptionsAndSelections #7711
 - MAGETWO-70464: Fix shipping address can use for billing #10130
 - MAGETWO-69913: magento/magento2 #9196 - Products ordered report doesn't show simple (child) products of configurable products #9908
@yarcowang
Copy link

yarcowang commented Nov 24, 2017

Em... I've just did a post in Linkedin Has anyone tried PHP builtin web server?. Is this patch resolving my problem? (Well, according to the README.md under phpserver, I thought it could work and I did some wrong somewhere... first time touching magento actually)
And for now, I'm using nginx+php-fpm, but I prefer php builtin server for development environment.

Updated:
It seems I need to visit http://localhost:30080/index.php/, but not http://localhost:30080/... with index.php, it works... Still didn't work, maybe I see the cache?

Updated:
The patch could work. Confirmed. And you should visit /index.php instead of /, or you will see an empty page. (to me)

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

Successfully merging this pull request may close these issues.

5 participants