Rewrite as a Svelte/Express app #56

Rich-Harris · 2017-03-14T16:26:50Z

https://sveltetechnology-ukcvrplota.now.sh

Svelte is mature enough that it makes sense to use it rewrite https://svelte.technology. Apart from testing out the development workflow, this has some tangible benefits:

the code is a lot more organised (much easier to include components in multiple pages, for example)
the site feels faster, because of client-side routing

Still very work-in-progress, lots of outstanding tasks:

plus whatever I forgot.

A few notes on developer experience:

The way CSS is generated (creating a component that imports all other components, bundling it with generate: 'ssr' then calling app.renderCss()) is a bit clunky. It means you have to maintain that App.html file as you add new routes, and you have to send all the CSS for the entire app (though maybe that's not a bad thing — dynamically adding CSS is often a CSP violation, and the CSS is tiny).
Because the CSS isn't generated dynamically, editing a component means that the unique hash gets out of date, and server-rendered CSS isn't applied to client-rendered components. Would be nice if there was a good way to generate the CSS dynamically while developing (but still serve the baked version in production) — otherwise you have to keep stopping and starting the npm run dev task
HMR would be nice!
Client-side routing could be a bit more declarative — though the trick is to do that without losing the ability to do things like this, which is hard to do if you go ultra-declarative with your routing setup. (Maybe that particular bit of logic belongs in the component itself, I don't know.)
There's probably some stuff that's unnecessarily duplicated between server-side and client-side routing. For now I like having them separate, but it could eventually seem like a chore, or (worse) bug-prone.

As we gather more of this sort of data on building these kinds of apps, maybe it'll become clearer what can be packaged up somehow in a way that improves the developer experience, in the way that Next.js improved the experience of building universal React apps.

Rich-Harris · 2017-03-15T03:48:14Z

I've deployed this even though there are tasks outstanding, as it's an improvement on the previous site, and it will hopefully mean feedback/bug reports.

Conduitry · 2017-03-15T03:58:40Z

I was actually just taking a look at this locally. One thing I saw (besides the missing roadtrip dependency, which I see you fixed) was a weird routing bug on Firefox. Start from a particular anchor on the guide page, then click to the REPL or blog, then press back - the last back doesn't work, I'm staying on whatever the non-guide page was. This works fine on Chrome and works fine if you don't start from an anchor on the guide page.

Rich-Harris · 2017-03-15T19:24:00Z

Huh, that is weird. What version of FF are you using? It seems to be okay for me on FF52 on OS X. Do you see any errors in the console when that happens?

Ryuno-Ki · 2017-03-15T19:09:44Z

shared/routes/Guide.html

+			blocked = true;
+
+			fn();
+			setTimeout( unblock, ms );


I wonder whether this could lead to a memory leak if the timeouts aren't cleared.

shouldn't do, why would it? The timeout is never meant to be cleared before it executes, and you obviously don't need to clear a timeout after it's been executed

Ryuno-Ki · 2017-03-15T19:11:46Z

shared/routes/Index.html

+		padding: 2em 1em;
+		/*background-color: #333;
+		color: #f4f4f4;*/
+		background-color: white;


Usually you use hex values for colours.

I defer to Emerson 😀

Ryuno-Ki · 2017-03-15T19:13:13Z

shared/routes/Index.html

+
+	.learn-svelte {
+		background-color: rgb(170,30,30);
+		color: white;


Usually you use hex values for colours.

Ryuno-Ki · 2017-03-15T19:15:11Z

shared/routes/Index.html

+	@media (min-width: 480px) {
+		.left, .right {
+			max-width: 32em;
+			margin: 0 auto;


You need to make it a block element (or take care to not add those classes to inline elements as that won't have an effect).

Ryuno-Ki · 2017-03-15T19:16:31Z

shared/routes/Repl/CodeMirror.html

+		bottom: 0;
+		padding: 0.5em;
+		background-color: rgb(255,220,220);
+		border-top: 1px solid rgb(200,0,0);


You usually use hex values for colours.

Ryuno-Ki · 2017-03-15T19:19:13Z

shared/routes/Repl/CodeMirror.html

+			// the component is actually in the DOM
+			setTimeout( () => {
+				this.editor.refresh();
+			});


What happens with setTimeout if you omit the second argument?

It defaults to 0

Ryuno-Ki · 2017-03-15T19:21:58Z

shared/routes/Repl/Viewer.html

+					toTeardown = component;
+					component = null;
+
+					setTimeout( () => { // necessary because `data` is not yet updated. TODO would be nice to fix this


Missing second argument?

Ryuno-Ki · 2017-03-15T19:24:50Z

shared/routes/Repl/index.html

+			<div class='controls'>
+				<button class='{{showGenerated ? "" : "active"}}' on:click='set({ showGenerated: false })'>input</button>
+				<button class='{{showGenerated ? "active" : ""}}' on:click='set({ showGenerated: true })'>output</button>
+				<button class='{{saving ? "active" : ""}}' on:click='save()'>{{saving ? 'saving' : 'save'}}</button>


Semantically those <button>s are of type „submit”.

I associate submit with forms, rather than event triggers. You could maybe argue the 'save' button is, but the others aren't, they're effectively tabs. With submit you have to prevent the event default — is there any real advantage to submit? If it's an a11y thing, maybe there's an aria attribute that would be more appropriate?

Ryuno-Ki · 2017-03-15T19:27:00Z

shared/routes/Repl/index.html

+		margin: 0 0.5em 0 0;
+		border-radius: 0.2em;
+		color: rgb(170,30,30);
+		border: 1px solid rgb(170,30,30);


Usually you use hex values for colours.

Ryuno-Ki · 2017-03-15T19:27:42Z

shared/routes/Repl/index.html

+
+	.controls button.active {
+		background-color: rgb(170,30,30);
+		color: white;


Usually you use hex values for colours.

Conduitry · 2017-03-15T19:49:06Z

I'm on Firefox 52 on Windows. I am getting a TypeError: t.state is null error when I go back from the non-guide page to the guide with the fragment. I'm also seeing that same error when I click on anchor links from within the guide page, but I guess that doesn't actually cause problems at that moment because the browser just follows it like a regular anchor hyperlink.

Rich-Harris · 2017-03-15T19:50:40Z

Huh, thought I'd fixed that in Rich-Harris/roadtrip@27778fa. Is this happening on the deployed site, or locally? Could it be that your dependencies are out of date?

Conduitry · 2017-03-15T19:53:46Z

This is on the live deployed site. But taking a look at the minified code in https://svelte.technology/bundle.js I don't see that change.

Rich-Harris · 2017-03-15T20:09:27Z

Curiouser and curiouser. I've redeployed, and it looks like bundle.js does now have that check, so hopefully that's the end of it...

Conduitry · 2017-03-15T20:14:49Z

All right, I'm no longer getting that console error - but the navigation still isn't working. When I go back from /repl to /guide#some-anchor I'm assuming the new check is now returning early from the popstate handler, but there is still something that has to happen on that popstate. (Should this conversation move over to roadtrip? 😃)

Rich-Harris · 2017-03-15T20:23:59Z

Ok, now I'm more freaked out that it is working for me! Now that you mention it does seem like something is missing from that code. Perils of trying to fix bugs at the end of the day.

Should this conversation move over to roadtrip?

Probably! And if you know of any alternative routers that use regular <a> tags instead of framework-specific stuff, and handle scroll history etc, then I'm all ears — I don't really want to be in the router business, I just couldn't find anything that either a) did nothing and had no opinions or b) did everything and had all the opinions

Rich-Harris/roadtrip#9

Conduitry · 2017-03-16T12:27:24Z

The div.error with the error messages in the CodeMirror component in the REPL are not displaying correctly. They're present in the DOM when they should be, but not visible on the page.

Conduitry · 2017-03-16T14:27:50Z

Everything under the src directory can be removed with this rewrite, correct?

Rich-Harris · 2017-03-16T14:37:59Z

Everything under the src directory can be removed with this rewrite, correct?

Yep, am just keeping it around so it's easier to copy and paste stuff if I need to... have updated the task list at the top of the page so we don't forget

Conduitry · 2017-03-16T20:11:03Z

So lol oops - just the other day I opened a PR against https://github.com/flagello/awesome-sveltejs to have the link to the REPL there be to https://svelte.technology/repl/?version=1&gist=c80ee9ec68fefa93617dfc40400851f5 instead of a particular version because that would grab the latest 1.x.x version and would redirect/rewrite the version param in the url - but that feature no longer seems to be working! Was that an intended feature before? Actually, the REPL isn't working at all right now? Nothing suspicious in the console, but svelte.js isn't being loaded from unpkg.

Edit for real this time - https://svelte.technology/repl works fine but https://svelte.technology/repl/ does not.

Rich-Harris · 2017-03-16T20:26:59Z

Ah, right — that makes sense. We probably just need to add an entry here

Conduitry · 2017-03-16T20:30:06Z

Some LESS snuck into the CSS here.

Rich-Harris · 2017-03-19T19:14:47Z

Merging this, will open separate issues for the remaining tasks

Rich-Harris added 16 commits March 13, 2017 15:36

start rebuilding as Svelte/Express app

13cd33e

add guide

07eed0b

implement some more nav stuff

55fc08c

keep built routes/components out of repo

31fa19d

fix stuff for now

0e047f1

sourcemaps

e5f99f4

fix scroll anchor positions

1966319

various

fca87c6

hmm

95012d8

remove unused files

0bc689e

repl mostly working

f4abac1

load initial example in REPL

267df96

fix SSR

420865d

fix fonts in repl viewer

9f7869c

fix links

23132f3

syntax highlighting

ed09a4a

Rich-Harris mentioned this pull request Mar 14, 2017

CSS bug in the REPL sveltejs/svelte#372

Closed

Rich-Harris added 13 commits March 14, 2017 18:36

activate guide JS, scroll correctly

71fc87a

remove guide JSON from repo

c26631c

remove repl button for a couple of snippets

99afc4e

fix z indexes

b85ed43

fix repl

7238a61

fix guide generation

26cb60c

fix blog links

6efb361

section highlighting

caccbd1

fix nav

e298308

smoother scrolling in guide

44d9653

guess passive event listeners are broken, huh

65849a0

fallback fonts

f2dc340

fix minor error

2bbad9a

update roadtrip

3074ebb

Ryuno-Ki reviewed Mar 15, 2017

View reviewed changes

ensure rollup is run before deployment

87d2a8e

Rich-Harris mentioned this pull request Mar 15, 2017

Handle hashchange driven popstate events correctly Rich-Harris/roadtrip#9

Closed

ensure curl is loaded before the bundle

b781683

Rich-Harris added 2 commits March 19, 2017 14:59

redirect old REPL links

9efbee7

fix css

1777e11

Rich-Harris merged commit e171295 into master Mar 19, 2017

Rich-Harris deleted the svelte-app branch March 19, 2017 19:14

Rich-Harris mentioned this pull request Mar 27, 2017

prehistoric version of uglify-js #54

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rewrite as a Svelte/Express app #56

Rewrite as a Svelte/Express app #56

Rich-Harris commented Mar 14, 2017 •

edited

Loading

Rich-Harris commented Mar 15, 2017

Conduitry commented Mar 15, 2017

Rich-Harris commented Mar 15, 2017

Ryuno-Ki Mar 15, 2017

Rich-Harris Mar 15, 2017

Ryuno-Ki Mar 15, 2017

Rich-Harris Mar 15, 2017

Ryuno-Ki Mar 15, 2017

Ryuno-Ki Mar 15, 2017

Ryuno-Ki Mar 15, 2017

Ryuno-Ki Mar 15, 2017

Rich-Harris Mar 15, 2017

Ryuno-Ki Mar 15, 2017

Ryuno-Ki Mar 15, 2017

Rich-Harris Mar 15, 2017

Ryuno-Ki Mar 15, 2017

Ryuno-Ki Mar 15, 2017

Conduitry commented Mar 15, 2017

Rich-Harris commented Mar 15, 2017

Conduitry commented Mar 15, 2017

Rich-Harris commented Mar 15, 2017

Conduitry commented Mar 15, 2017

Rich-Harris commented Mar 15, 2017

Conduitry commented Mar 16, 2017

Conduitry commented Mar 16, 2017

Rich-Harris commented Mar 16, 2017

Conduitry commented Mar 16, 2017 •

edited

Loading

Rich-Harris commented Mar 16, 2017

Conduitry commented Mar 16, 2017

Rich-Harris commented Mar 19, 2017

Rewrite as a Svelte/Express app #56

Rewrite as a Svelte/Express app #56

Conversation

Rich-Harris commented Mar 14, 2017 • edited Loading

Rich-Harris commented Mar 15, 2017

Conduitry commented Mar 15, 2017

Rich-Harris commented Mar 15, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Conduitry commented Mar 15, 2017

Rich-Harris commented Mar 15, 2017

Conduitry commented Mar 15, 2017

Rich-Harris commented Mar 15, 2017

Conduitry commented Mar 15, 2017

Rich-Harris commented Mar 15, 2017

Conduitry commented Mar 16, 2017

Conduitry commented Mar 16, 2017

Rich-Harris commented Mar 16, 2017

Conduitry commented Mar 16, 2017 • edited Loading

Rich-Harris commented Mar 16, 2017

Conduitry commented Mar 16, 2017

Rich-Harris commented Mar 19, 2017

Rich-Harris commented Mar 14, 2017 •

edited

Loading

Conduitry commented Mar 16, 2017 •

edited

Loading