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

extractScripts and extractStyles don't work as expected #29

Closed
manu-st opened this issue Mar 30, 2017 · 9 comments
Closed

extractScripts and extractStyles don't work as expected #29

manu-st opened this issue Mar 30, 2017 · 9 comments

Comments

@manu-st
Copy link
Contributor

manu-st commented Mar 30, 2017

I cloned the repo, did npm install at the root and then launched the server as node example/index.js. When I inspect the source of the rendered page in the browser, both style and scripts are still at their original position in the view instead of being respectively at the top and bottom.

Is this expected?
@soarez
Copy link
Owner

soarez commented Mar 30, 2017

No, it's not expected. But I also can't reproduce this issue.

The script block is placed at the end of the rendered HTML and the style block is yanked up inside the <head> element, as seen below:

$ curl http://localhost:3000/
<!DOCTYPE html>
<html lang="en">
<head>
  <meta charset="UTF-8">
  <title>Example</title>

  
  <style>
  .page-message { color: blue }
</style>
</head>
<body>
  <header>
    
    <h1 class="page-title">Example</h1>

  </header>

  
This is part of the body.








<h1 class="page-message">This is a message</h1>



  <footer>
    
    <h1>This is the footer</h1>


  </footer>

  
  <script>
  // Script content!
</script>
</body>
</html>

@soarez soarez closed this as completed Mar 30, 2017
@manu-st
Copy link
Contributor Author

manu-st commented Mar 30, 2017

It is definitely not doing it for me. Can you give me directions at where I should be looking at?
I'm getting:

<!DOCTYPE html>
<html lang="en">
<head>
  <meta charset="UTF-8">
  <title>Example</title>

  
  
</head>
<body>
  <header>
    
    
<h1 class="page-title">Example</h1>


  </header>

  

This is part of the body.



<style>
  .page-message { color: blue }
</style>


<script>
  // Script content!
</script>

<h1 class="page-message">This is a message</h1>



  <footer>
    
    
<h1>This is the footer</h1>



  </footer>

  
  
</body>
</html>

@manu-st
Copy link
Contributor Author

manu-st commented Mar 30, 2017

I'm testing on Windows with node.js version 6.10.1

@manu-st
Copy link
Contributor Author

manu-st commented Mar 30, 2017

I checked and the regular expression for the script extraction doesn't work.

@manu-st manu-st mentioned this issue Mar 30, 2017
Closed
@soarez soarez reopened this Mar 30, 2017
@soarez
Copy link
Owner

soarez commented Mar 30, 2017

This is really strange, I'm on macOS, tested with the same version of Node, but still can't replicate. Will try with a VM as soon as possible.

@manu-st
Copy link
Contributor Author

manu-st commented Mar 31, 2017 via email

soarez pushed a commit that referenced this issue Mar 31, 2017
Update test instructions in Readme #29
c24943f
@soarez
Copy link
Owner

soarez commented Mar 31, 2017

Sorry, the instructions were outdated. Run the tests with npm test.

@soarez
Copy link
Owner

soarez commented Mar 31, 2017

#30 might make it work. The test suite fails on windows because of the different EOL used by windows. Until the test suite is fixed, I recommend configuring git and your editor to use linux style line endings i.e. \n instead of \r\n.

@soarez soarez closed this as completed Mar 31, 2017
@manu-st
Copy link
Contributor Author

manu-st commented Mar 31, 2017

Will do. Thanks!

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

Successfully merging a pull request may close this issue.

Fix regexp so that it matches style and script manu-st/express-ejs-layouts
2 participants
@soarez @manu-st