-
-
Notifications
You must be signed in to change notification settings - Fork 310
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 pwa configuration, current release only works with default values #384
Conversation
@@ -20,11 +20,12 @@ module.exports = (nextConfig = {}) => { | |||
webpack, | |||
buildId, | |||
dev, | |||
config: { distDir = '.next', pwa = {}, pageExtensions = ['tsx', 'ts', 'jsx', 'js', 'mdx'], experimental = {} } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the pwa part in here resulted in not taking the pwa config at all.
let basePath = options.config.basePath | ||
if (!basePath) basePath = '/' | ||
const basePath = options.config.basePath || '/' | ||
const { pwa = {} } = nextConfig |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the real fix, the rest is a bit of code cleanup.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!!! It works 🎉
This is almost the same thing as #372 . Except that it also does some code cleanup. |
Line 337 in 9d28c3b
the comment here explained why I believe the Next.js team prefer let nextConfig be nextConfig and handle lib config as the second argument. |
@shadowwalker could we take this fix to be able to release a patch for the current version? In parallel we should start on introducing the breaking change following best practices as discussed in #381. |
Need this PR approved ASAP. Package is broken until approved |
resolves #383
@shadowwalker could you make a
5.5.6
release including this fix?I have tested the changes on my nextjs project using.