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

feat(builder): followSymlinks option to allow for symlinks #6368

Merged
merged 8 commits into from
Sep 29, 2019

Conversation

SnirShechter
Copy link

Types of changes

  • Bug fix (a non-breaking change which fixes an issue)
  • New feature (a non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Description

This PR adds a build.followSymlinks boolean(default: false), and passes it to the builder's glob function inside resolveFiles().

This allows for following symlinks inside layouts/middleware/pages/store, thus including them in the app.

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly. (PR: #)
  • I have added tests to cover my changes (if not applicable, please state why)
  • All new and existing tests are passing.

@codecov-io
Copy link

codecov-io commented Sep 5, 2019

Codecov Report

Merging #6368 into dev will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##              dev    #6368   +/-   ##
=======================================
  Coverage   95.65%   95.65%           
=======================================
  Files          79       79           
  Lines        2692     2692           
  Branches      696      696           
=======================================
  Hits         2575     2575           
  Misses        101      101           
  Partials       16       16
Flag Coverage Δ
#e2e 100% <ø> (ø) ⬆️
#fixtures 50.7% <ø> (ø) ⬆️
#unit 92.34% <ø> (ø) ⬆️
Impacted Files Coverage Δ
packages/config/src/config/build.js 100% <ø> (ø) ⬆️
packages/builder/src/builder.js 99.64% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 686720f...4c74a3a. Read the comment docs.

@pimlie
Copy link

pimlie commented Sep 10, 2019

@SnirShechter Thanks for this pr. Would it maybe be possible to combine the tests with an existing fixture? Adding it to basic or with-config should be fine.
The reason for this is that every new fixture adds about ~10seconds to the test time. So we try to only add new fixtures for incompatible configs.

Also note that it seems you are using a different emailaddress in your local git config then for github. If you want you can either add that other emailaddress to your github account (Settings -> Emails) or correct the emailaddress & do a force-push (old commits will still be accessible though).

@SnirShechter
Copy link
Author

@SnirShechter Thanks for this pr. Would it maybe be possible to combine the tests with an existing fixture? Adding it to basic or with-config should be fine.
The reason for this is that every new fixture adds about ~10seconds to the test time. So we try to only add new fixtures for incompatible configs.

Also note that it seems you are using a different emailaddress in your local git config then for github. If you want you can either add that other emailaddress to your github account (Settings -> Emails) or correct the emailaddress & do a force-push (old commits will still be accessible though).

Hey, sorry for the really late reply. I'm on it.

@pi0 pi0 requested review from clarkdo and pimlie September 20, 2019 07:59
Copy link

@pimlie pimlie left a comment

Choose a reason for hiding this comment

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

LGTM. The only thing I was thinking about is to maybe make it possible for a user to set any glob option, but I think we probably shouldnt do that as changing a setting like absolute or realpath could result in unexpected behaviour in nuxt.

@SnirShechter
Copy link
Author

It is something worth thinking of, but basically followSymlinks is (through my point of view) much more than a simple glob setting, since it allows separation of pages in different folders. For me and my company, this means that we can integrate one project into another, when built right.

Thanks for the approval 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants