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

Context filename #11

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Context filename #11

wants to merge 2 commits into from

Conversation

tmpfs
Copy link

@tmpfs tmpfs commented Jan 27, 2017

This fixes an issue where setting options.root to the filename context (https://github.com/reshape/layouts/blob/master/lib/index.js#L8) would override use of the context filename on subsequent requests to resolve layouts when no root option is given.

For example, by assigning the context filename to options.root (when no root option is specified) it would never be called again - the root is set to the first file processed and used by all subsequent files.

This ensures all layouts are resolved correctly when no root option is given.

Also updated the .gitignore for the code coverage output directory.

@jescalan
Copy link
Member

Awesome, thank you for the contribution! Is there any chance you could add a test case to ensure this doesn't end up being missed by any future changes?

@tmpfs
Copy link
Author

tmpfs commented Feb 5, 2017

Hey,

I have looked into adding a test case, however I am not sure how to go about configuring the test specs/fixtures as we need to be able to pass multiple context filenames in a single pass to be sure that the dirname() for subsequent requests is used as the root.

Suggestions?

@jescalan
Copy link
Member

jescalan commented Feb 6, 2017

So what was the original way you were using it that inspired you to make this fix in the first place? Running something like this is a pretty reliable way to ensure that it's working now 😁

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.

2 participants