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

feat: Friendly prerender message + remove UglifyJS from SSR build #195

Merged
merged 5 commits into from
Jul 8, 2017

Conversation

rkostrzewski
Copy link
Collaborator

@rkostrzewski rkostrzewski commented Jul 5, 2017

This closes #178

screen shot 2017-07-05 at 16 52 48

Copy link
Collaborator

@thangngoc89 thangngoc89 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is freaking awesome !!!
Best DX ever <3 <3

But we might want to consider add some info about prerender (why? cons?) and point user to that page. Because currently, there are no info about this on README

@rkostrzewski
Copy link
Collaborator Author

Heh, previously I've had See https://github.com/developit/preact-cli#prerender for further information. but I was too lazy to write it 😆

@lukeed
Copy link
Member

lukeed commented Jul 5, 2017

Looks great @rkostrzewski ! 😻

Quick question though: Why was uglify removed on SSR? UglifyJS was mangling the maps, ergo making tracing impossible?

@rkostrzewski
Copy link
Collaborator Author

@lukeed that's one of the reasons there other being that chief said so 😆, right @developit ?

@lukeed
Copy link
Member

lukeed commented Jul 5, 2017

@rkostrzewski Lol 😂 Just went thru that Slack. Looks like Chief said that because of the source map issues!

@hassanbazzi
Copy link
Member

I notice this suggests people should check for window. I always just use this:

https://www.npmjs.com/package/window-or-global

It also makes SSR work for a bunch of features that need global, and it doesn't break even if you run things like eventListeners on global.

@rkostrzewski
Copy link
Collaborator Author

@thangngoc89 I've added Pre-rendering part to the README.md let me know if this is what you had in mind and I'll be merging this

@thangngoc89
Copy link
Collaborator

@rkostrzewski LGTM

@rkostrzewski rkostrzewski added this to the 1.4.0 milestone Jul 8, 2017
@rkostrzewski rkostrzewski merged commit 72f979d into master Jul 8, 2017
@rkostrzewski rkostrzewski deleted the feature/prerender-error-help branch July 8, 2017 20:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Prerender error and workarounds
4 participants