-
Notifications
You must be signed in to change notification settings - Fork 394
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
[RFC] Some improvements and breaking changes #36
Labels
Comments
This was referenced Nov 15, 2017
stereobooster
added a commit
that referenced
this issue
Nov 16, 2017
stereobooster
added a commit
that referenced
this issue
Nov 16, 2017
stereobooster
added a commit
that referenced
this issue
Nov 16, 2017
stereobooster
added a commit
that referenced
this issue
Nov 16, 2017
stereobooster
added a commit
that referenced
this issue
Nov 16, 2017
In the noscript issue, I tried for react and it does add the styles to noscript. Should I create a PR? |
@ryan-ds17 yes totally |
stereobooster
added a commit
that referenced
this issue
Nov 16, 2017
stereobooster
added a commit
that referenced
this issue
Nov 16, 2017
stereobooster
added a commit
that referenced
this issue
Nov 16, 2017
stereobooster
added a commit
that referenced
this issue
Nov 16, 2017
stereobooster
added a commit
that referenced
this issue
Nov 16, 2017
stereobooster
added a commit
that referenced
this issue
Nov 16, 2017
stereobooster
added a commit
that referenced
this issue
Nov 16, 2017
stereobooster
added a commit
that referenced
this issue
Nov 18, 2017
stereobooster
added a commit
that referenced
this issue
Nov 18, 2017
stereobooster
added a commit
that referenced
this issue
Nov 18, 2017
stereobooster
added a commit
that referenced
this issue
Nov 18, 2017
stereobooster
added a commit
that referenced
this issue
Nov 18, 2017
stereobooster
added a commit
that referenced
this issue
Nov 18, 2017
Last call before I release this beast. Closing issue there is a PR |
stereobooster
added a commit
that referenced
this issue
Nov 18, 2017
stereobooster
added a commit
that referenced
this issue
Nov 18, 2017
stereobooster
added a commit
that referenced
this issue
Nov 18, 2017
stereobooster
added a commit
that referenced
this issue
Nov 18, 2017
stereobooster
added a commit
that referenced
this issue
Nov 18, 2017
stereobooster
added a commit
that referenced
this issue
Nov 18, 2017
stereobooster
added a commit
that referenced
this issue
Nov 19, 2017
stereobooster
added a commit
that referenced
this issue
Nov 19, 2017
stereobooster
added a commit
that referenced
this issue
Nov 19, 2017
stereobooster
added a commit
that referenced
this issue
Nov 19, 2017
stereobooster
added a commit
that referenced
this issue
Nov 19, 2017
stereobooster
added a commit
that referenced
this issue
Nov 19, 2017
stereobooster
added a commit
that referenced
this issue
Nov 19, 2017
stereobooster
added a commit
that referenced
this issue
Nov 19, 2017
stereobooster
added a commit
that referenced
this issue
Nov 19, 2017
stereobooster
added a commit
that referenced
this issue
Nov 19, 2017
stereobooster
added a commit
that referenced
this issue
Nov 19, 2017
stereobooster
added a commit
that referenced
this issue
Nov 19, 2017
stereobooster
added a commit
that referenced
this issue
Nov 20, 2017
stereobooster
added a commit
that referenced
this issue
Nov 20, 2017
stereobooster
added a commit
that referenced
this issue
Nov 20, 2017
stereobooster
added a commit
that referenced
this issue
Nov 20, 2017
stereobooster
added a commit
that referenced
this issue
Nov 20, 2017
stereobooster
added a commit
that referenced
this issue
Nov 20, 2017
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
path/index.html
vspath.html
From the ticket #22. I do not see any sense to generate
path.html
for/path
route instead ofpath/index.html
. This convention was copied from react-snapshot.After the change
react-snap
will generatepath/index.html
for both:/path
and/path/
Preload resources
As of now, this config does two things:
window.snapStore[<path>]
<link rel="preload" as="image">
)Need to fix two things:
renamewindow.snapStore
towindow.SERVER_DATA
as in c-r-a instruction https://github.com/facebookincubator/create-react-app/blob/master/packages/react-scripts/template/README.md#injecting-data-from-the-server-into-the-pagewebpack 2+ code splitting hack
There is a hack to make webpack code splitting work. Because we assume usage with c-r-a it should be enabled by default. But before enabling it by default need to fix it - use
<link rel="preload"
instead of script tags404 page
As of now react-snap by default generates 404 page, which I consider a good practice, but it actually doesn't make sense to generate 404 if you do not use routing (like react-route or similar), I mean if you have only one page.
So I consider changing this behavior and generate 404 page if there is more than one page discovered by the crawler or listed in include.
Another thing that I was wondering is how to detect that
404.html
is actually a "not found page". How to make sure programmer didn't forget to handle this case?The only option I came up is: check if the title of the page contains the string "404". It can be in any position. Also, the nice benefit is that it will possible to track that no other page is "404".
Proper exit status for errors
support errors in minimalcssmoved to separate ticketCheck if target directory contains
200.html
, and if it does fail with an error message and non-zero exit code.Also, exit with non-zero exit code if any error occurred during generation and stop generation. As of now react-snap reports error in console, but continues to generate pages.
The text was updated successfully, but these errors were encountered: