Validates source on project creation, closes #1257#1269
Conversation
|
|
||
| export function* createProject() { | ||
| yield put(projectCreated(generateProjectKey())); | ||
| yield put(validatedSource('javascript', [])); |
There was a problem hiding this comment.
So this approach definitely works, but kind of as a second-order effect, because the compiledProjects saga consumes this action and initiates compilation in response to it.
My preference is to keep action dispatches as close as possible to an unvarnished record of what actually happened (somebody clicked a button, some async operation finished, etc.) In this case we are saying we’ve finished validating a source, but in fact we haven’t (this intuition is strengthened, I think, by the fact that we have to arbitrarily pick a language to say we have validated).
My proposal would be: instead of adding a new dispatch into the flow, simply make the system react more robustly to the actions that are already being dispatched. In particular, I think the compiledProjects saga should consume the PROJECT_CREATED action and compile the project in response.
LMK what you think!
There was a problem hiding this comment.
Yeah, makes sense! This solution indeed felt slightly wrong with the language hard-coded.
The thinking was that, given that the source is a brand new Popcode, it can safely be assumed valid. I was after that second-order effect :) Much clearer to simply compile the project, updated.
| yield all([throttle(100, 'VALIDATED_SOURCE', validatedSource)]); | ||
| yield all([ | ||
| throttle(100, 'VALIDATED_SOURCE', validatedSource), | ||
| takeEvery('APPLICATION_LOADED', validatedSource), |
There was a problem hiding this comment.
This is fairly inside baseball, but I think we actually want to consume PROJECT_CREATED here—APPLICATION_LOADED will trigger different behavior in the projects saga depending on the initial application state (snapshot/gist ID in the URL, project rehydrated from local storage, etc.), and in some circumstances the saga will kick off a full validation. The case when it doesn’t do a full validation is exactly the case when it dispatches PROJECT_CREATED.
As a bonus, that’ll makes the console work right after you click “New Project”, which it currently does not—a closely related bug!
There was a problem hiding this comment.
Oops! Right yes makes sense, tho didn't realize that re: "New Project"
outoftime
left a comment
outoftime
left a comment
There was a problem hiding this comment.
Now we’re cooking with gas!!
2 participants
This PR closes issue 1257.
It's also my first Popcode PR 🎉 Validating the source on project creation made sense to me but feedback is welcome as I learn more about the code base :)