-
Notifications
You must be signed in to change notification settings - Fork 237
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
Add default session data #501
Conversation
Thanks @mikeshawdev, I've been doing something very similar but in a more hacky way. This is very helpful. |
What about naming the file I'd like to separately add a file for storing global data, and intend to call it |
start.js
Outdated
@@ -17,6 +17,21 @@ if (!envExists) { | |||
.pipe(fs.createWriteStream(path.join(__dirname, '/.env'))) | |||
} | |||
|
|||
// Create template session data defaults file if it doesn't exist | |||
const sessionDefaultsDirectory = path.join(__dirname, '/app/data') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if this should be named defaultDataDirectory
- so that in the future if we reference files in this directory we can reuse it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 for this idea, I think that would be more suitable
start.js
Outdated
const sessionDefaultsExists = fs.existsSync(sessionDefaultsFile) | ||
|
||
if (!sessionDefaultsExists) { | ||
console.log('Creating template session data defaults file') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we creating a template file, or just the defaults file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, we should strip "template" from the sentence as we're creating from a template, not creating a template
start.js
Outdated
|
||
if (!sessionDefaultsExists) { | ||
console.log('Creating template session data defaults file') | ||
if (!fs.existsSync(sessionDefaultsDirectory)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We will be adding a separate global-data.js
file also in this directory. I wonder if we can check for the directory separately than checking for the session defaults file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should refactor in that PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I can refactor this later.
ff6ecf2
to
8618f5b
Compare
Looks good to me 👍 |
If provided, the app will try to merge data from a defaults file with data from the session. Defaults will only apply if there isn't a corresponding value already in the session, so data can be overridden within the prototype if necessary.
8618f5b
to
ebdf461
Compare
By @mikeshawdev
If provided, the app will try to merge data from a defaults file with data from the session. Defaults will only apply if there isn't a corresponding value already in the session, so data can be overridden within the prototype if necessary.