Serialize preloaded data for initial page and serve to client #87
Conversation
|
Glory be! The tests finally pass on Travis, now that it's not calling |
| .then(id => { | ||
| assert.equal(id, 42); | ||
| }); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Awesome stuff!
Is this test supposed to be removed?
You know this better than me, but is it a good idea to have both package-lock.json and yarn.lock in the same repository?
There was a problem hiding this comment.
ack! well spotted, thanks — must have got lost when I was trying to merge some stuff. have added it back.
Bloody lockfiles. Removing yarn.lock
There was a problem hiding this comment.
Can you explain the reason for removing lock files? I always commit mine. (Yarn being the preferred tool in my builds.)
There was a problem hiding this comment.
Lock files are used to get consistent installs across machines. Having 2 separate lock files in the same repository violates this out of the gate. Having either yarn.lock or package-lock.json in the repository is a good idea, though.
There was a problem hiding this comment.
Did you mean to say Sapper only supports NPM? That would make more sense. But there's nothing wrong with committing Yarn and NPM lock files—it is just more effort.
Since Yarn has been pretty reliable almost since inception, I barely noticed the release of NPM 5, and just now had to go read up on the difference, including the lock files.
If this ticket is ever closed yarnpkg/yarn#3614 then it may be that Yarn folks won't need to fret about frameworks, etc., that rely solely on NPM, but until then, we should state it more explicitly in the docs that Sapper relies on NPM; use Yarn at your own expense (YMMV).
ALTERNATIVELY, you can commit both lock files and explicitly support both, but give the Sapper team a little more work to do. I could easily see y'all doing the first, with a well-worded explanation somewhere.
This PR looks bigger than it otherwise would, because it includes a bunch of semi-related test stuff, and adds a helper for printing nice startup errors (which could be made more general if necessary) to help move people away from
%sapper.main%