Skip to content
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

Pass elder config to constructor #52

Closed

Conversation

decrek
Copy link
Contributor

@decrek decrek commented Oct 9, 2020

This PR adds the ability to pass Elder config options to the Elder constructor and thus bypassing/overruling cosmiconfig finding the elder config file. For a full reasoning why I want that see #28.

Resolves #28.

…se instead of process.cwd(). Let it default to process.cwd()
… flexibility. Merge it with existing or default configuration.
# Conflicts:
#	package-lock.json
#	src/Elder.ts
#	src/externalHelpers.ts
#	src/hooks.ts
#	src/routes/routes.ts
#	src/utils/getConfig.ts
#	src/utils/getHashedSvelteComponents.ts
#	src/utils/svelteComponent.ts
#	src/utils/types.ts
#	src/utils/validations.ts
#	yarn.lock
@decrek
Copy link
Contributor Author

decrek commented Oct 9, 2020

@nickreese 1 test is still failing and I have no clue why. Could you have a look / point me in the right direction?
Also here I removed some strings being passed to getConfig. I think this was a leftover from previously being able to pass a context to getConfig that was removed when moving to v1?

@@ -74,12 +74,12 @@ class Elder {

shortcodes: ShortcodeDefs;

constructor({ context, worker = false }) {
constructor({ context, worker = false, configOptions = {} }) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@decrek as you mentioned, these config options would ideally be on the same nesting level as context and worder, so maybe do this:

constructor({ context, worker = false, ...configOptions })

then configOptions will always be an object with all the other options (and {} if no other options are given).

@nickreese
Copy link
Contributor

@decrek I will try and get to this PR this week. Had some pressing health issues come up.

@nickreese
Copy link
Contributor

@decrek @jbmoelker no idea how to use the new gh tool lol. Won't let me push my changes here. Opened a new PR with your changes and a working fix.

#55

@nickreese nickreese closed this Oct 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Serverless Elder rendering
3 participants