This updates our `yarn next` command to leverage react v18 by default and removes the need for the test require hook/config modifying when testing react 18. There are some fixtures we need to investigate react 18 support in follow-ups:
- `test/integration/client-navigation-a11y`
- `test/integration/critical-css`
- `test/integration/custom-error-page-exception`
- `test/integration/font-optimization`
- AMP specific tests
* Read the proper page file from either pages directory or from node_modules (inernal pages like _app, _document)
* Only reading page runtime when `reactRoot` is enabled, reduce time for react 17 apps
* fix the dynamic routing of middleware
* add middleware to dynamicRoutes of routes-manifest
* remove unused import
* fix middleware routing with static paths
* update manifest version test
* prevent to match with api route using regex
* use iterator instead of generator
* do not use Iterator
* fix type
* fix type
* remove unused import
* apply the fix for support colons
Co-authored-by: JJ Kasper <jj@jjsweb.site>
Partially implements #31317 and #31506. There're also some trade-offs made with this PR: since we can't know if a certain runtime will be used or not beforehand, we have to start both runtime compilers (Node.js and Edge) and then generate entrypoints correspondingly.
Note that with this PR, the global runtime is still required to use the per-page runtime.
## Bug
- [ ] Related issues linked using `fixes #number`
- [x] Integration tests added
- [ ] Errors have helpful link attached, see `contributing.md`
## Feature
- [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.
- [ ] Errors have helpful link attached, see `contributing.md`
## Documentation / Examples
- [ ] Make sure the linting passes by running `yarn lint`
## Bug
- [x] Related issues linked using `fixes #number`
- [x] Integration tests added
- [ ] Errors have helpful link attached, see `contributing.md`
Fixes#34030.
The PR is still WIP as the test case hasn't been added, help or change is welcome.
cc @no-ya @ijjk
Co-authored-by: JJ Kasper <22380829+ijjk@users.noreply.github.com>
This commit adds a development mode warning in the console
if you try to include <script> tags in next/head, e.g.
```
<Head>
<script async src="..." />
</Head>
```
The warning message explains that this pattern will not
work well with Suspense/streaming and recommends using the
next/script component instead.
TODO in follow-up PR: add same warning for stylesheets, etc
## Feature
- [x] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
- [x] Integration tests added
- [x] Documentation added
- [x] Errors have helpful link attached, see `contributing.md`
fixes#32178
the `inline-script-id` eslint rule crashed when encountering a `JSXSpreadAttribute`. this pr fixes that, and also handles `id` being passed via the spreaded object.
## Bug
- [x] Related issues linked using `fixes #number`
- [x] ~~Integration~~ Unit tests added
- [ ] Errors have helpful link attached, see `contributing.md`
## Feature
- [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.
- [ ] Errors have helpful link attached, see `contributing.md`
## Documentation / Examples
- [ ] Make sure the linting passes by running `yarn lint`
We currently have inconsistencies when working with URLs in the Edge Functions runtime, this PR addresses them introducing a warning for inconsistent usage that will break in the future. Here is the reasoning.
### The Browser
When we are in a browser environment there is a fixed location stored at `globalThis.location`. Then, if one tries to build a request with a relative URL it will work using that location global hostname as _base_ to construct its URL. For example:
```typescript
// https://nextjs.org
new Request('/test').url; // https://nextjs.org/test
Response.redirect('/test').headers.get('Location'); // https://nextjs.org/test
```
However, if we attempt to run the same code from `about:blank` it would not work because the global to use as a base `String(globalThis.location)` is not a valid URL. Therefore a call to `Response.redirect('/test')` or `new Response('/test')` would fail.
### Edge Functions Runtime
In Next.js Edge Functions runtime the situation is slightly different from a browser. Say that we have a root middleware (`pages/_middleware`) that gets invoked for every page. In the middleware file we expose the handler function and also define a global variable that we mutate on every request:
```typescript
// pages/_middleware
let count = 0;
export function middleware(req: NextRequest) {
console.log(req.url);
count += 1;
}
```
Currently we cache the module scope in the runtime so subsequent invocations would hold the same globals and the module would not be evaluated again. This would make the counter to increment for each request that the middleware handles. It is for this reason that we **can't have a global location** that changes across different invocations. Each invocation of the same function uses the same global which also holds primitives like `URL` or `Request` so changing an hypothetical `globalThis.location` per request would affect concurrent requests being handled.
Then, it is not possible to use relative URLs in the same way the browser does because we don't have a global to rely on to use its host to compose a URL from a relative path.
### Why it works today
We are **not** validating what is provided to, for example, `NextResponse.rewrite()` nor `NextResponse.redirect()`. We simply create a `Response` instance that adds the corresponding header for the rewrite or the redirect. Then it is **the consumer** the one that composes the final destination based on the request. Theoretically you can pass any value and it would fail on redirect but won't validate the input.
Of course this is inconsistent because it doesn't make sense that `NextResponse.rewrite('/test')` works but `fetch(new NextRequest('/test'))` does not. Also we should validate what is provided. Finally, we want to be consistent with the way a browser behaves so `new Request('/test')` _should_ not work if there is no global location which we lack.
### What this PR does
We will have to deprecate the usage of relative URLs in the previously mentioned scenarios. In preparation for it, this PR adds a validation function in those places where it will break in the future, printing a warning with a link that points to a Next.js page with an explanation of the issue and ways to fix it.
Although middleware changes are not covered by semver, we will roll this for some time to make people aware that this change is coming. Then after a reasonable period of time we can remove the warning and make the code fail when using relative URLs in the previously exposed scenarios.
Noscript is not required for Image that are loaded immediately
## Bug
- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Errors have helpful link attached, see `contributing.md`
## Feature
- [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
- [ ] Related issues linked using `fixes #number`
- [x] Integration tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.
- [ ] Errors have helpful link attached, see `contributing.md`
## Documentation / Examples
- [ ] Make sure the linting passes by running `yarn lint`
Co-authored-by: Steven <229881+styfle@users.noreply.github.com>
This fixes broken links in the eslint output by removing the trailing full stop.
It also makes the formatting of (the output of) the various rules consistent.
## Documentation / Examples
- [x] Make sure the linting passes by running `yarn lint`
> I don't think this is a bug, nor a feature, nor is it really documentation.
> It's just a small nuisance that I bumped into and felt compelled to fix.
> I went with documentation as that seems the closest match
## What does this pull request do?
The elslint output of `eslint-plugin-next` contains useful links to the documentation about the various rules.
Unfortunately, on most (but not all) rules, those links are immediately followed by a full stop (`.`).
The terminal (or any parser) has no way of knowing that the full stop is not part of the URL.
So it includes it and clicking the link leads to a 404 on the nextjs.org website.
This PR fixes that by removing the full stop.
## But a final full stop is better grammar
I considered alternatives (such as [a zero-width space character](https://en.wikipedia.org/wiki/Zero-width_space#Prohibited_in_URLs)) in case the final full stop was part of the style guide or something.
However, as I went through the eslint rules, I notices that the messages for various rules were formatted inconsistently.
Some with final full stop, some without.
As such, I made the all consistent with this structure:
> [message]. See: [url]
I feel this is a better solution than using the zero-width space as these sort of invisible characters
in code can be a red flag that something fishy is going on.
I submit this pull request in the hope it will be useful, and a positive contribution to a project I have a great deal of appreciation for.
That being said, I fully understand if people would consider this a non-issue.
When an `a` tag is used to link to an internal page, but the `target="_blank"` attribute is present, ESLint should not report it as an error and should not enforce using `next/link`.
Fixes#28547
## Bug
- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Errors have helpful link attached, see `contributing.md`
## Feature
- [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.
- [ ] Errors have helpful link attached, see `contributing.md`
## Documentation / Examples
- [ ] Make sure the linting passes by running `yarn lint`
**Note**: This PR is applying again changes landed #31935 that were reverted from an investigation.
This PR fixes#30398
By default Next will polyfill some fetch APIs (Request, Response, Header and fetch) only if fetch is not found in the global scope in certain entry points. If we have a custom server which is adding a global fetch (and only fetch) at the very top then the rest of APIs will not be polyfilled.
This PR adds a test on the custom server where we can add a custom polyfill for fetch with an env variable. This reproduces the issue since next-server.js will be required without having a polyfill for Response which makes it fail on requiring NextResponse. Then we remove the code that checks for subrequests to happen within the **sandbox** so that we don't need to polyfill `next-server` anymore.
The we also introduce an improvement on how we handle relative requests. Since #31858 introduced a `port` and `hostname` options for the server, we can always pass absolute URLs to the Middleware so we can always use the original `nextUrl` to pass it to fetch. This brings a lot of simplification for `NextURL` since we don't have to consider relative URLs no more.
## Bug
- [x] Related issues linked using `fixes #number`
- [x] Integration tests added
- [x] Errors have helpful link attached, see `contributing.md`
People have been reporting on https://github.com/vercel/next.js/pull/30973 that the `no-server-import-in-page` eslint rule is reporting false positives for `_middleware` files inside sub-page folders.
Unlike `_document`, we can have multiple `_middleware` files.
Fixes#32121
This PR fixes#30398
By default Next will polyfill some fetch APIs (Request, Response, Header and fetch) only if fetch is not found in the global scope in certain entry points. If we have a custom server which is adding a global fetch (and only fetch) at the very top then the rest of APIs will not be polyfilled.
This PR adds a test on the custom server where we can add a custom polyfill for fetch with an env variable. This reproduces the issue since next-server.js will be required without having a polyfill for Response which makes it fail on requiring NextResponse. Then we remove the code that checks for subrequests to happen within the **sandbox** so that we don't need to polyfill `next-server` anymore.
The we also introduce an improvement on how we handle relative requests. Since #31858 introduced a `port` and `hostname` options for the server, we can always pass absolute URLs to the Middleware so we can always use the original `nextUrl` to pass it to fetch. This brings a lot of simplification for `NextURL` since we don't have to consider relative URLs no more.
## Bug
- [x] Related issues linked using `fixes #number`
- [x] Integration tests added
- [x] Errors have helpful link attached, see `contributing.md`
Previously `response.cookie(name, value, options)` would mutate the passed in `options` which lead to unexpected behaviour as described in #31666.
This PR clones the `options` argument before mutating it.
## Bug
- [x] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Errors have helpful link attached, see `contributing.md`
## Feature
- [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
- [x] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.
- [ ] Errors have helpful link attached, see `contributing.md`
## Documentation / Examples
- [x] Make sure the linting passes by running `yarn lint`
Since `localhost` is actually an alias for `127.0.0.1` that points to loopback, we should take that into consideration at `NextURL` when we handle local URLs.
The implementation is based on [is-localhost-url](https://github.com/Kikobeats/is-localhost-url); I added some tests over local URLs variations present at the library to ensure other variations are working fine.
Additionally, I refactor some things over the code to avoid doing the same twice and added some legibility that is always welcome when you are working with URLs stuff.
closes https://github.com/vercel/next.js/issues/31533
The rule [`no-html-link-for-pages`](https://nextjs.org/docs/messages/no-html-link-for-pages) will incorrectly flag an `<a>`-tag intended to download a local asset. This PR adds an exception to the rule for any anchor element with a `download` attribute.
Co-authored-by: JJ Kasper <22380829+ijjk@users.noreply.github.com>
closes: https://github.com/vercel/next.js/issues/30353
According with spec, `'about:client'` is the default value is the user doesn't provide it.
It needs to add a test there, looks like there no unit tests for these classes 🤔
* add isEqualNode function
* add test
* trying to make integration test work
* revert
* Update test/unit/is-equal-node.unit.test.js
Co-authored-by: Steven <steven@ceriously.com>
* Revert "revert"
This reverts commit d67b9971068d18efcf839666a3a17619fd914fc3.
* Fix tests
* Use TS for unit test
* Revert waitfor
* Start tests with "should"
* Fix lint
* Use cloneNode()
Co-authored-by: Eric Biewener <eric.biewener0@walmart.com>
Co-authored-by: Steven <steven@ceriously.com>
## Bug
- [x] Related issues linked using `fixes #number`
- [x] Integration tests added
- [ ] Errors have helpful link attached, see `contributing.md`
Fixes#30430
There's some more discussion in the issue, but in summary:
- web `Headers` implementation combines all header values with `', '`
- For `Set-Cookie` headers, you're supposed to set them as separate values, not combine them
- web `Headers` forbids the use of `Cookie`, `Set-Cookie` and some more headers, so they don't have custom implementation for those, and still joins them with `,`
- We currently just split them using `split(',')`, but this breaks when the header contains a date (expires, max-age) that also includes a `,`
I used this method to split the Set-Cookie header properly: https://www.npmjs.com/package/set-cookie-parser#splitcookiestringcombinedsetcookieheader as suggested [here](https://github.com/whatwg/fetch/issues/973#issuecomment-559678813)
I didn't add it as a dependency, since we only needed that one method and I wasn't sure what the process is for adding dependencies, so I just added the method in the middleware utils
Make the `no-unwanted-polyfillio` rule respond to the `next/script` component as well as the `script` tag.
## Feature
- [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
- [ ] Related issues linked using `fixes #number`
- [x] ~Integration~Unit tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.
- [ ] Errors have helpful link attached, see `contributing.md`
This PR adds support for [Middleware as per RFC ](https://github.com/vercel/next.js/discussions/29750).
## Feature
- [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.
- [ ] Errors have helpful link attached, see `contributing.md`
## Documentation / Examples
- [ ] Make sure the linting passes
This refactor is the first of a few changes to support "classic" (two-part)
streaming. This one should be a noop that doesn't actually change the behavior.
It re-organizes the way that functions are wrapped in Document Head/NextScript
so anything that will be part of the second flush can be separated out from the
first flush. It also adds the structure for a useMaybeDeferContent hook, but
currently always assumes that nothing should be deferred.
The next PRs will actually implement streaming.
* Preserve `next-env.d.ts` line ending
Prevent next from changing already existing line ending on
`next-env.d.ts` for no good reason
* Update comparison
Co-authored-by: Steven <steven@ceriously.com>
* Update checks and add tests
* update test
Co-authored-by: Steven <steven@ceriously.com>
Co-authored-by: jj@jjsweb.site <jj@jjsweb.site>
Fixes#28635
## Bug
- [x] Related issues linked using `fixes #number`
- [x] Integration tests added
- [x] Errors have helpful link attached, see `contributing.md`
* Move unit tests to one folder
* Migrate unit tests to TypeScript
* add test types to lint
* Ensure ts(x) tests are run with util
* Add tsx extension to jest config
* bump
### Changes
#### Feature
* Adopt `React.lazy` into `next/dynamic`, enable it when `options.suspense` is `true`
* Support `next/dynamic` with `suspense=true` in SSR and SSG
#### Tests
| Scenario | Case | Behavior |
|:----:|:----:|:----:|
| basics | react 17 or 18 by default | dev/build error or pass |
| blocking rendering | `reactRoot: true` + `concurrentFeatures: false` | dev/build pass |
| concurrent rendering | `reactRoot: true` + `concurrentFeatures: true` | dev/build pass |
## Feature
- [x] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
- [ ] Related issues linked using `fixes #number`
- [x] Integration tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.
- [ ] Errors have helpful link attached, see `contributing.md`
Remove unnecessary `React.Suspense` checks, as the minimum supported version is `react@>=17.0.1`. Also removes some files from the `react-18` integration test that are no longer necessary as of #26664.
I recently received the "Props must be returned as a plain object" error when I accidentally dropped an array directly inside `props`. This is a trivial error to fix once you know what you did, but I spent good 30 minutes looking in all the wrong places. I hope that this slight clarification in the error message would save someone from wasting their time if they make a similar mistake like myself.
Fixes#19862
Avoid executing `webpack` property on `loadableGenerated` of loadable component compiled from `next/dynamic` when `require.resolveWeak` is unavailable due to jest runtime missing `require.resolveWeak`.
## Bug
- [x] Related issues linked using `fixes #number`
- [x] unit tests added
The current `<Image />` component does not fallback gracefully when JavaScript is disabled in the client / browser.
You can test this with the [official Next/Image example](https://csb-4k0kr-p8ya8f304.vercel.app/), by disabling JavaScript in the browser's DevTools. Video demo: https://streamable.com/frkvw9
This PR aims to fix this behaviour by using `<noscript></noscript>` tags to conditionally display a standard `<img>` element using the `props` passed to `<Image />` when JavaScript is disabled.
For browser sessions where JavaScript is enabled, this will not cause an increase in network requests, so there should be no downside.
One area where this PR is a bit "hacky" is that it uses a negative `margin-top` to counteract `sizerStyle.paddingTop`. From what I can tell, `sizerStyle.paddingTop` is generated on the server side, where we can not know ahead of time whether JavaScript is enabled in the browser - hence why I've opted for this solution.
Fixes#19223Fixes#21214
I tried the preset provided at `packages/next-plugin-storybook` but it was raising error due to an [unsafe negation](https://eslint.org/docs/rules/no-unsafe-negation) in the `preset.js` file.
I added a test to show the error:
```
● next-plugin-storybook filterModuleRules › should filter module rules correctly
TypeError: rule.test.test is not a function
48 | if (!rule.test instanceof RegExp) return true
49 | // use Next.js' built-in CSS
> 50 | if (rule.test.test('hello.css')) {
| ^
51 | return false
52 | }
53 | // use next-babel-loader instead of storybook's babel-loader
at filter (../packages/next-plugin-storybook/preset.js:50:21)
at Array.filter (<anonymous>)
at Object.filterModuleRules (../packages/next-plugin-storybook/preset.js:46:28)
at Object.<anonymous> (unit/webpack-config-overrides.test.js:12:36)
```
This PR fixes a bug where `next/babel` would accidentally enable development transforms for a production build (`next build`).
This is tested by the two updated unit tests (which removed a workaround for this bug, and one now properly enables dev transforms).
---
Fixes#18929Fixes#19001
x-ref #19046
x-ref #17032
This reverts #18921 and ensures that the Babel runtime is only inlined as an absolute path when using PnP as before, but then including the correction this resolution as implemented by @merceyz only in the PnP cases, while keeping the diff to a minimum.
This PR removes the modern mode experiment because:
- It does not yield meaningful bundle size wins when compared to other initiatives we've taken
- It's not compatible with webpack 5 (which we're upgrading to)
- It's currently broken and causes most apps to malfunction
- There's no champion currently owning the experiment
We can re-introduce this in the future when we'd like to make it a default for all Next.js apps.
Note: **Next.js still supports Differential Loading (`nomodule`) and does it by default.** This PR strictly removes the experimental modern _syntax_, and does not disable our existing modern/legacy polyfilling.
---
Fixes#19200Fixes#18960Fixes#14707Fixes#14465
This replaces the seen set with a newly instantiated map of refs per
value in the array. This corrects the behavior that the refs map passed
into isSerializable contains only values along the path to the current
value: previously, because the refs map was shared among all values in
the array, this would trigger on instances such as:
```js
const x = [];
isSerializableProps('/', 'test', { arr: [x, [x]] });
```
... where the reference is shared but there is no cycle formed, as the
presence of elements is disjoint.
Fixes#18228.
Next.js plugins like `@next/mdx` inject additional webpack loaders to compile files, but they omit the necessary loader for Fast Refresh to work.
Instead of making these files deopt out of Fast Refresh, we can automatically detect and inject the loader in these cases.
Fixes#13574
This updates the named regexes output in the `routes-manifest` and the associated `routeKeys` to not use any non-word characters as this breaks the named regexes e.g. `"Invalid regular expression: "^/(?<data\-provider\-id>[^/]+?)(?:/)?$"`
x-ref: https://github.com/zeit/now/pull/4355
This pull request refactors our TypeScript preflight check in preparation for dropping the `fork-ts-checker-webpack-plugin` plugin.
This will make reviewing the subsequent PR much easier.
---
There is no behavior change, so the existing test should cover this adequately.
* Allow `@babel/preset-typescript` to be configured
Some usages, e.g. [getting parameter decorators to work properly](https://github.com/WarnerHooh/babel-plugin-parameter-decorator/pull/19), require configuring `@babel/preset-typescript`.
This commit adds that functionality to `next/babel`.
* Add unit test for babel/preset allowing to pass options to @babel/preset-typescript
* Add integration test for onlyRemoveTypeImports
* Update babel dependencies
* Update to compatible typescript version and fix types
* Fix linting and run pre-nccing
* Update size-limit test
* Add additional tests
* Re-Apply delta to be calculated using -262
Co-authored-by: JJ Kasper <jj@jjsweb.site>
* Verify GS(S)P Serializability
* Add support for cyclic refs
* Add unit tests
* Test for error in development mode
* Fix prerender preview tests
* Fix gssp preview tests
* fix 2x test cases
* Add desired test
* fix some more tests
* Fix route manifest expect
* Fix test expects
* Test that `getServerSideProps` does not error in production
* Test that getStaticProps is not checked in production
* Test serialization check during build
* Fix export detection for serverless
* Update test/unit/is-serializable-props.test.js
Co-Authored-By: JJ Kasper <jj@jjsweb.site>
Co-authored-by: JJ Kasper <jj@jjsweb.site>
* Rename getServerProps to getServerSideProps
* Remove unstable_ prefix from new methods
* Add error when legacy methods are detected
* Add legacy methods for babel transform
* Add unstable_getServerSideProps also
* Apply suggestions from code review
Co-Authored-By: Joe Haddad <joe.haddad@zeit.co>
* Update types import
Co-authored-by: Joe Haddad <timer150@gmail.com>
* WIP: rename get static paths
* Adjust logic
* Add warning
* rename constant
* Check for error when using old method
* Add friendly error message
* Test for message of invalid keys
* replace index with dots
* Drop Re-Exported iSSG Imports
This pull request adds loader support for the following export form:
```js
export { getStaticParams as unstable_getStaticParams } from '../../lib/docs/page';
export { getStaticProps as unstable_getStaticProps } from '../../lib/docs/page';
```
This will now result in correct detection and removal.
* Add unit tests for next page config behavior
* Test extra function is not dropped
* Catch-all Route Support
* Add SSR tests
* Add additional test
* Add unit tests
* Put the feature behind a flag
* Apply suggestions from code review
Co-Authored-By: JJ Kasper <jj@jjsweb.site>
* Fix test
* Add transparent JSX optimization.
* fix duplicate React import
* fix React not being imported when only a single Fragment node is present in a source module
* remove babel-plugin-react-require
* Fix JSX optimization for CommonJS source files.
* Add error message for duplicate slug name within a dynamic path
* Update based on feedback
Co-authored-by: Tim Neutkens <tim@timneutkens.nl>
* WIP - committed with no-verify - progress
Co-authored-by: Tim Neutkens <tim@timneutkens.nl>
* Remove old test placeholder
* Add test for re-used names
