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

No css/scripts referenced inside index html #1016

Closed
MartB opened this Issue Oct 15, 2018 · 9 comments

Comments

Projects
None yet
3 participants
@MartB
Contributor

MartB commented Oct 15, 2018

My mainpage shows:

<link href="" rel="stylesheet"><link href="" rel="stylesheet"><link href="" rel="stylesheet">
<script src="/config"></script><script src="" defer="defer"></script><script src="" defer="defer"></script>

Eventhough webpack seems to run without any errors.
I suspect this is related to the recent changes to webpack.

Any pointers?
Edit: found a fix that works for me.

@MartB MartB changed the title from No css referenced inside index html to No css/scripts referenced inside index html Oct 15, 2018

MartB added a commit to MartB/codimd that referenced this issue Oct 15, 2018

@SISheogorath

This comment has been minimized.

Member

SISheogorath commented Oct 16, 2018

Can you check your webpack version that is used? And what version of CodiMD are you running? When it's latest master, please make sure you successfully upgraded to Webpack version 4.

@davidmehren

This comment has been minimized.

Contributor

davidmehren commented Oct 16, 2018

I can't reproduce this in the current master.
When I try to apply @MartB s fix, everything breaks because the links look like <link href="[object Object]" rel="stylesheet">

@MartB

This comment has been minimized.

Contributor

MartB commented Oct 16, 2018

daheck how are you installing webpack?

i nuked node_modules and ran npm install and it will not work.

Edit:
webpack package.json

 "_args": [
    [
      "webpack@4.20.2",
      "/opt/hackmd"
    ]
  ],

running latest master too.

@davidmehren

This comment has been minimized.

Contributor

davidmehren commented Oct 16, 2018

Correction: I can reproduce this.
It seems to be our version definition of "html-webpack-plugin": "^4.0.0-alpha" (note the ^).
This allows version 4.0.0-beta.2 to be installed, which needs the changed template.
My yarn.lock has prevented this upgrade, so I could not reproduce the error.

@MartB Do you want to amend your PR with html-webpack-plugin pinned to 4.0.0-beta.2 and an updated yarn.lock?

@MartB

This comment has been minimized.

Contributor

MartB commented Oct 16, 2018

@davidmehren sure!
Edit quick question why is everything > 10 not compatible?

MartB added a commit to MartB/codimd that referenced this issue Oct 16, 2018

@MartB

This comment has been minimized.

Contributor

MartB commented Oct 16, 2018

Done i hope i did it right heh.

Btw:
If everyone is using yarn we should get rid of the npm install step, or provide a package-lock.json for everyone using npm install.

@SISheogorath SISheogorath added this to the 1.3.0 release milestone Oct 16, 2018

@davidmehren

This comment has been minimized.

Contributor

davidmehren commented Oct 16, 2018

I think we should remove the ^ in the version range, so while html-webpack-plugin is in beta, we don't get more surprises like this one.

Regarding Node 10: Some dependencies (I think it was uws) use native code and are not compatible with Node 10.

I'll do a PR to change the docs shortly.

@SISheogorath

This comment has been minimized.

Member

SISheogorath commented Oct 16, 2018

Much more important and breaks: scrypt.

But we already got a solution by ccoenen (don't want to mention him unneeded), just needs to be adapted for CodiMD. As soon as we fixed that, we should be compatible to node 10.

And about yarn vs. npm install: See #943 which already introduces such a change :)

But I should rewrite the setup script entirely as I realized lately.

Edit: Just as hint: uws was already dropped in 1.2.1 Scratch that

@MartB

This comment has been minimized.

Contributor

MartB commented Oct 16, 2018

The only thing i have to say about uws is = https://www.reddit.com/r/node/comments/91kgte/uws_has_been_deprecated/

Im using the normal websocket module and i experienced no issues.
However im not using it on a large scale deployment.

@BackToTopic
I will change it.

SISheogorath added a commit that referenced this issue Oct 16, 2018

Merge pull request #1017 from MartB/master
Fix #1016: webpack include defect for scripts and header files.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment