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

fix: import next.config.js in Windows #159

Merged
merged 12 commits into from
Feb 2, 2024
Merged

Conversation

luwes
Copy link
Collaborator

@luwes luwes commented Jan 31, 2024

related #151

it's fixed, the issue seems to be that Windows path.resolve() behaves differently than Unix.
it did require a cwd() argument while Unix didn't it seemed.

path.resolve(cwd(), file);

I also added pathToFileURL to make sure it's a valid URL for JS dynamic import() because it expects an URL, not a path.
nodejs/node#34765

@luwes luwes self-assigned this Jan 31, 2024
@luwes luwes changed the title chore: add windows ci test fix: add windows ci test Jan 31, 2024
@luwes luwes changed the title fix: add windows ci test fix: import next.config.js in Windows Jan 31, 2024
@codecov-commenter
Copy link

codecov-commenter commented Jan 31, 2024

Codecov Report

Attention: 15 lines in your changes are missing coverage. Please review.

Comparison is base (2ae04bf) 57.94% compared to head (828cf71) 58.43%.
Report is 4 commits behind head on main.

Files Patch % Lines
src/components/utils.ts 80.00% 7 Missing ⚠️
src/config.ts 61.53% 4 Missing and 1 partial ⚠️
src/components/video.tsx 40.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #159      +/-   ##
==========================================
+ Coverage   57.94%   58.43%   +0.49%     
==========================================
  Files          29       30       +1     
  Lines        2166     2216      +50     
  Branches      110      120      +10     
==========================================
+ Hits         1255     1295      +40     
- Misses        909      916       +7     
- Partials        2        5       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@heff heff left a comment

Choose a reason for hiding this comment

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

lgtm, one suggestion

next.config.js Outdated Show resolved Hide resolved
@luwes luwes merged commit 969d6e8 into muxinc:main Feb 2, 2024
4 checks passed
@luwes luwes deleted the test-windows branch February 2, 2024 15:45
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.

3 participants