Skip to content

Make next a peer dependency #45

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

Merged
merged 1 commit into from
Nov 17, 2020
Merged

Make next a peer dependency #45

merged 1 commit into from
Nov 17, 2020

Conversation

ehmicky
Copy link

@ehmicky ehmicky commented Nov 17, 2020

Fixes #26 and #8
Also part of #25

This makes next a peer dependency, which allows users to define which Next.js version they prefer (among the versions we support).
It a site is not using next, an error message is shown.
This also changes the version range of next from ^9.5.3 to >=9.5.3 so that Next.js 10 users can use this, although support is experimental and we will print a warning message (to be done in separate PR).

Testing this turns out to be quite complicated. Making require('next') fail in automated tests is pretty hard to achieve without mocking require() which could bring its own sets of problems. Tests need next as a devDependency to run properly, so require('next') is guaranteed to succeed otherwise, even when the current directory changes (since require() is based on __filename instead). So I think we might need to leave out automated testing for this specific validation check (and also for the upcoming Next.js versions checks).

@ehmicky ehmicky added the type: feature code contributing to the implementation of a feature and/or user facing functionality label Nov 17, 2020
@ehmicky ehmicky self-assigned this Nov 17, 2020
@ehmicky ehmicky force-pushed the feat/next-peer-dependency branch 2 times, most recently from 546ecf6 to 9d2c594 Compare November 17, 2020 19:00
@ehmicky ehmicky mentioned this pull request Nov 17, 2020
@ehmicky ehmicky force-pushed the feat/next-peer-dependency branch from 9d2c594 to 3866870 Compare November 17, 2020 19:01
}
}

module.exports = { validateNextUsage }

Choose a reason for hiding this comment

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

curious why exported as object instead of just module.exports = validateNextUsage

Copy link
Author

@ehmicky ehmicky Nov 17, 2020

Choose a reason for hiding this comment

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

If this file exports new functions in the future, we don't need to update the files importing validateNextUsage().
I can change it to a default export if you prefer though. Which one do you prefer?

Choose a reason for hiding this comment

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

no preference! just used to the default non-object export consistent throughout NoN but i'm totally cool with being future-proof here, i think it makes sense if you think we might put another func/helper in here. up to you!

Copy link
Author

@ehmicky ehmicky Nov 17, 2020

Choose a reason for hiding this comment

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

Let's go with consistency and use a default export. Fixed 👍

@@ -46,6 +46,11 @@ module.exports = {

const nextConfigPath = await findUp('next.config.js')
if (nextConfigPath !== undefined) {
// We cannot load `next` at the top-level because we validate whether the
// site is using `next` inside `onPreBuild`.

Choose a reason for hiding this comment

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

🖖

@ehmicky ehmicky force-pushed the feat/next-peer-dependency branch from 3866870 to 4acffcf Compare November 17, 2020 19:26
@ehmicky ehmicky merged commit 97cc69a into main Nov 17, 2020
@ehmicky ehmicky deleted the feat/next-peer-dependency branch November 17, 2020 19:41
serhalp pushed a commit that referenced this pull request Apr 5, 2024
* feat: add cache set for route handler

* chore: increase timeout as tests are flaky

* chore: update
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature code contributing to the implementation of a feature and/or user facing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Better error handling when the site does not use Next.js
2 participants