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.

Sign up for GitHub

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

Jump to bottom

Fix(config): sections not rendering correctly #1133

Merged
merged 12 commits into from
Sep 13, 2018
Merged

Fix(config): sections not rendering correctly #1133

merged 12 commits into from
Sep 13, 2018

Conversation

elevatebart
Copy link
Collaborator

@elevatebart elevatebart commented Sep 11, 2018 •
edited
Loading

Closes #1132

Fixes and simplifies configuration on windows

@elevatebart elevatebart changed the title Fix: sections not rendering correctly Fix(config): sections not rendering correctly Sep 11, 2018
@codecov-io
Copy link

codecov-io commented Sep 11, 2018 •
edited
Loading

Codecov Report

Merging #1133 into master will decrease coverage by 0.05%.
The diff coverage is 87.5%.

Impacted Files Coverage Δ
loaders/utils/getComponentFiles.js 100% <100%> (ø) ⬆️
scripts/schemas/config.js 87.14% <66.66%> (-1.1%) ⬇️

@elevatebart elevatebart self-assigned this Sep 11, 2018
Fix: use fast-glob instead of glob
fb90cd3 
This fixes issues on windows

Closes #1132
sapegin
sapegin requested changes Sep 11, 2018
View reviewed changes
loaders/utils/getComponentFiles.js Outdated Show resolved Hide resolved
loaders/utils/getComponentFiles.js Outdated Show resolved Hide resolved
Barthelemy Ledoux and others added 5 commits September 11, 2018 15:24
fix unit tests
7121195
Refact: simplify map and reduce
e6dc420
@elevatebart
use node-glob back for incertitudes on ignore issues
51aa72b
@elevatebart
Fix(defaults): fix win32 defaults
829334c 
Instead of removing case management in getComponents files,
I choose to do it here.
This way, I am not impacted by the bugs surrounding node-glob
with nocase on Windows platforms
@elevatebart
revert: updating node-glob
c56f0b4
@sapegin
Copy link
Member

sapegin commented Sep 12, 2018

Looks like you need to revert test case changes, otherwise looks good! I'd also add a HACK: in front of the comment ;-)

@elevatebart
Copy link
Collaborator Author

Hello @sapegin,
I changed the test cases because I have no choice. The removal of glob.hasMagic has changed the behaviour of getComponentFiles. It now always goies through the glob. This means that the array of non existent files is not returned anymore if we pass it to the function. I changed the paths to point to real test files. And the tests are passing again.

Codecov was actually complaining about another file: config.js. I added a new branch, the windows specific glob without testing it. Since I do not think we plan on making tests pass on windows, I simply ignored the coverage for his line.

Bart

@sapegin
Copy link
Member

sapegin commented Sep 13, 2018

Ah, I see! That's OK ;-)

@sapegin sapegin merged commit 0e6e95a into master Sep 13, 2018
@sapegin sapegin deleted the fix-sectionresolving branch September 13, 2018 12:54
@styleguidist-bot
Copy link
Collaborator

🎉 This PR is included in version 7.3.8 🎉

The release is available on:

Your semantic-release bot 📦🚀

@ThorHelms
Copy link

Just FYI: It seems that this breaks Styleguidist on Windows when using uppercase 'Components' folder, as the resources will be requested with lowercase 'components' which doesn't exist.
For now I've been lazy and simply reverted to 7.3.7.

@elevatebart
Copy link
Collaborator Author

Hello @ThorHelms, what can you share your version of windows, the command prompt you are using and your version of nodejs? I would like to investigate this issue.

@elevatebart
Copy link
Collaborator Author

@ThorHelms another question, did you use the Components folder before 7.3.7 ?

@ThorHelms
Copy link

I am using:
Windows 10 Pro, version 1803
PowerShell 5.1
NodeJS 8.11.2

I used the Components folder with Styleguidist 7.2.2, and after an update to 7.3.8 it no longer worked.

sapegin pushed a commit that referenced this pull request Oct 8, 2018
@elevatebart
Fix: Regression: component patterns return nothing on Windows (#1133)
4726932
@dbrelovsky dbrelovsky mentioned this pull request Jan 11, 2019
Closed
This was referenced Apr 7, 2021
Open
Open
Open
Open
Open
Open
This was referenced May 6, 2021
Open
Open
Open
This was referenced May 25, 2021
Open
Open
Open
This was referenced Aug 17, 2021
Open
Open
This was referenced Sep 12, 2021
Open
Open
Open
@Bhanditz Bhanditz mentioned this pull request May 16, 2022
Open
@kopax kopax mentioned this pull request May 17, 2022
Open
@snyk-bot snyk-bot mentioned this pull request May 17, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sapegin sapegin sapegin approved these changes

Assignees

@elevatebart elevatebart

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@elevatebart @codecov-io @sapegin @styleguidist-bot @ThorHelms