-
Notifications
You must be signed in to change notification settings - Fork 86
fix: resolve next-server from next app directory and not from plugin #2059
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
LekoArts
merged 34 commits into
main
from
fix/use-next-directory-when-resolving-server-for-handler
May 4, 2023
Merged
Changes from all commits
Commits
Show all changes
34 commits
Select commit
Hold shift + click to select a range
8d8d8ae
fix: resolve next-server from next app directory and not from plugin
pieh e7ae048
fix: update api handler
pieh 15c1bdd
chore: export type from server module
pieh 4b64c17
fix: server.spec.ts
pieh 1933824
chore: move next server throwing to runtime
pieh e06157d
fix: restore trying to resolve from plugin and not appdir
pieh ffae6a2
fix: use relative import paths not absolute
pieh d50c047
fix: server.spec.ts (again)
pieh c5a4afd
fix: index.spec.ts
pieh 48468bc
chore: diff cleanup
pieh 33c28ae
fix: handle function config parsing as well
pieh be8bc6e
fix: adjust import
pieh caef767
fix: server.spec.ts (again vol 2)
pieh 56aa4f7
test: add integration test
pieh 1e48952
Merge remote-tracking branch 'origin/main' into fix/use-next-director…
pieh 22e2fa2
test: log npm version
pieh 6e229c9
test: install newer npm/node
pieh 74ab70e
test: bump timeout for setup
pieh e4fadf4
test: ensure test page is ssr
pieh 1774f47
refactor: get rid of unneded new helper
pieh bbff3a9
chore: cleanup some debugging logs
pieh ade1a2e
fix: add fallback in case findModuleBase will be false
pieh 610d73f
refactor: use one-parameter object for makeHandler functions
pieh 917d3ce
Merge remote-tracking branch 'origin/main' into fix/use-next-director…
pieh 1b01485
Merge remote-tracking branch 'origin/main' into fix/use-next-director…
pieh 04f3164
refactor: don't rely on MODULE_NOT_FOUND for lack of advanced API rou…
pieh 92dcf3a
Update packages/runtime/src/templates/server.ts
pieh 66a9670
fix: streamline no-shadow handling
pieh 6e4d78a
Merge branch 'main' into fix/use-next-directory-when-resolving-server…
LekoArts 8f71783
chore: automatic linting
LekoArts 9efa6cb
chore: fix linting
LekoArts a92dbac
chore: post-commit linting :rolleyes:
LekoArts 767dca9
Merge branch 'main' into fix/use-next-directory-when-resolving-server…
LekoArts 40c6469
Merge branch 'main' into fix/use-next-directory-when-resolving-server…
LekoArts File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
name: Next Runtime Integration Tests | ||
|
||
on: | ||
pull_request: | ||
push: | ||
branches: [main] | ||
|
||
concurrency: | ||
group: ${{ github.head_ref || github.run_id }} | ||
cancel-in-progress: true | ||
|
||
jobs: | ||
build: | ||
name: Integration tests | ||
runs-on: ubuntu-latest | ||
|
||
steps: | ||
- uses: actions/checkout@v2 | ||
- name: Installing with LTS Node.js | ||
uses: actions/setup-node@v2 | ||
with: | ||
node-version: 18 | ||
check-latest: true | ||
- name: Install netlify-cli and npm | ||
run: npm install -g netlify-cli npm | ||
- name: NPM Install | ||
run: npm install | ||
- name: Run integration tests | ||
run: npm run test:integration |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 not really integration test - it's really more e2e, but "e2e" is somewhat taken by tests we have copied from Next - so I went with integration for now, but maybe we just need to agree on how to refer to e2e tests that are not copied from next and I would love to adjust naming in this PR to reflect that
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.
Would it make sense to rename the Next tests 'upstream' tests?
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.
Yeah, I think it make sense, but at this point I'd rather ship those as-is and do renaming in separate standalone PR to not keep the fix hostage