Initial service worker #140
Conversation
| return { | ||
| name: "consts-plugin", | ||
| async resolveId(id) { | ||
| if (id !== "consts:") { |
There was a problem hiding this comment.
I'm ending both these new plugins with : to signify they're something other than node_modules. Open to better suggestions.
| output: { | ||
| dir: "dist", | ||
| format: "amd", | ||
| sourcemap: true, | ||
| entryFileNames: "[name]-[hash].js", | ||
| entryFileNames: "[name].js", |
There was a problem hiding this comment.
We don't want service worker to have a hash. This also removes the hash from bootstrap, but we always inline it so it's ok. I mentioned this issue to the Rollup folks rollup/rollup#2585 (comment)
| input: "src/bootstrap.ts", | ||
| input: { | ||
| bootstrap: "src/bootstrap.ts", | ||
| sw: "src/sw/index.ts" |
There was a problem hiding this comment.
Why is the SW a separate endpoint instead of using the same trick we are using for the Worker?
There was a problem hiding this comment.
The worker way gives us a hashed file name 😢
| } | ||
| return Object.entries(consts) | ||
| .map( | ||
| ([key, value]) => `export const ${key} = ${JSON.stringify(value)};` |
There was a problem hiding this comment.
Couldn’t this just be
export ${JSON.stringify(consts)}?
There was a problem hiding this comment.
Nah, that's invalid syntax. You could do export default ${JSON.stringify(consts)}, but then usage can't be minified or tree-shaken.
There was a problem hiding this comment.
TIL. I thought when you do
const a = 1;
const b = 2;
export {a, b};that the export is an object literal. It is not :(
| }; | ||
|
|
||
| private _gameChangeSubscribers = new Set<GameChangeCallback>(); | ||
| private _awaitingGameTimeout: number = -1; | ||
| private _stateService?: Remote<StateService>; |
There was a problem hiding this comment.
Why did you move this from props to an instance variable?
There was a problem hiding this comment.
It was in state previously. Updating state causes a re-render, but we never want to update rendering when this value changes. We update rendering when game changes, not when the means to create a game become available.
src/services/preact-canvas/index.tsx
Outdated
|
|
||
| this._awaitingGameTimeout = setTimeout(() => { | ||
| this.setState({ awaitingGame: true }); | ||
| }, 1000); |
There was a problem hiding this comment.
Move these kind of numbers to constants.ts?
| @@ -0,0 +1,12 @@ | |||
| { | |||
There was a problem hiding this comment.
Is needed (as discussed in person)
| @@ -11,5 +11,6 @@ | |||
| "moduleResolution": "node", | |||
| "baseUrl": "./", | |||
| "types": [] | |||
| } | |||
| }, | |||
| "exclude": ["src/sw/**/*"] | |||
| @@ -0,0 +1 @@ | |||
| import "../missing-types"; | |||
Ready for review!
The service worker will install during dev. Add
?no-swto the end of the page URL to disable it.I spent a while trying to find a good place for an "update available" message, but then I thought: can we avoid it? In this PR I don't show a message, but if the user presses "start" when there's a update available, it'll perform the update then start the game.
I refactored our loading state as part of this. We were giving too much away by disabling/dimming the start button. The user can press start whenever they want, we only have to admit we aren't ready if they beat us to it. I implemented that flow in this PR.