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

feat(nextjs): update to Next.js 13.3.0 #16130

Merged
merged 1 commit into from
Apr 12, 2023

Conversation

jaysoo
Copy link
Member

@jaysoo jaysoo commented Apr 5, 2023

This PR updates the installed version of Next.js to 13.3.0 and migrates existing workspaces.

We will hold off on migrating existing workspaces to 13.3 until we can put together a better migration experience for those using Storybook (they need to go to v7).

Also includes a few fixes. See below.

Use Storybook v7 for React/Next.js

Storybook v7 is stable now, and v6 is no longer maintained. Next.js 13.2 requires v7 to work, so it will be the default moving forward unless the user passes --storybook7Configuration=false when generating Storybook config (i.e. nx g @nrwl/react:storybook-configuration --storybook7Configuration=false).

Note: Another PR just landed to update Storybook to v7 for new projects. No changes needed here anymore.

Disable type checks by default (app/layout types are incorrect)

This only applies when appDir is enabled in next.config.js.

The Next.js type generation has a bug where the paths are wrong inside a monorepo. The PR is merged but waiting for a release (vercel/next.js#47534).

Once released we can re-enable the type-check by default. In the meantime the user can update next.config.js config to enable it if the patch is released before we update.

Generate libraries with separate RSC entry

Add a server-only entry for libraries generated with @nrwl/next:lib to help with libraries that export both client and server components. So users can import client-only components using deep import path. See: #15830

e.g.

Say our library my-lib exported both RSC and client components in index.ts.

// my-lib/src/index.ts
export { MyClientOnlyComponent } from './my-client-only-component';
export { MyServerOnlyComponent } from './my-server-only-component';

Then importing this in a client component will error out.

Fails:

'use client';
import { MyClientOnlyComponent } from '@acme/my-lib';

export function ParentComponent() {
  return <MyClientOnlyComponent />;
}

But if we separate the two components into separate entry files.

// my-lib/src/index.ts
export { MyClientOnlyComponent } from './my-client-only-component';

// my-lib/src/server.ts
export { MyServerOnlyComponent } from './my-server-only-component';

Then importing from client entry file will work in both server and client components.

Works:

'use client';
import { MyClientOnlyComponent } from '@acme/my-lib';

export function ParentComponent() {
  // Works because the main entry no longer has server-only components
  return <MyClientOnlyComponent />;
}

Also works:

import { MyClientOnlyComponent } from '@acme/my-lib';
import { MyServerOnlyComponent } from '@acme/my-lib/server';

export async function SomePage() {
  // Can render client component from server component
  return (
    <MyClientOnlyComponent/>
    <MyServerOnlyComponent/>
  );
}

This isn't an Nx issue because it's coming from @next/swc:

@vercel
Copy link

vercel bot commented Apr 5, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
nx-dev ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 12, 2023 2:46pm

Copy link
Contributor

@ndcunningham ndcunningham left a comment

Choose a reason for hiding this comment

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

LGTM

@jaysoo jaysoo force-pushed the feat/nextjs-13-2-update branch 2 times, most recently from 8476786 to e2ea7a4 Compare April 6, 2023 19:51
@@ -47,6 +47,19 @@ export function createFiles(host: Tree, options: NormalizedSchema) {
host.delete(`${options.projectRoot}/package.json`);
}

if (options.additionalImportPaths) {
for (const additionalImportPath of options.additionalImportPaths) {
Copy link
Member Author

Choose a reason for hiding this comment

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

This is just creating an empty file for additional import paths (e.g. src/server.ts for RSC). Other generators using this generator should fill them in with more useful content.

@jaysoo jaysoo force-pushed the feat/nextjs-13-2-update branch from e2ea7a4 to e0a1650 Compare April 6, 2023 19:55
@jaysoo jaysoo force-pushed the feat/nextjs-13-2-update branch from e0a1650 to b03383c Compare April 6, 2023 21:22
@jaysoo jaysoo force-pushed the feat/nextjs-13-2-update branch from b03383c to 2935d9c Compare April 6, 2023 23:33
@jaysoo jaysoo requested a review from Coly010 as a code owner April 6, 2023 23:33
@jaysoo jaysoo force-pushed the feat/nextjs-13-2-update branch from 2935d9c to a484dd4 Compare April 7, 2023 00:02
@jaysoo jaysoo changed the title feat(nextjs): update to Next.js 13.2.4 feat(nextjs): update to Next.js 13.3.0 Apr 7, 2023
@jaysoo jaysoo force-pushed the feat/nextjs-13-2-update branch from a484dd4 to 2019f0a Compare April 11, 2023 15:40
Comment on lines 91 to 71
if (options.additionalImportPaths) {
for (const additionalImportPath of options.additionalImportPaths) {
c.paths[`${options.importPath}/${additionalImportPath}`] = [
joinPathFragments(
options.projectRoot,
`./src/${additionalImportPath}.${options.js ? 'js' : 'ts'}`
),
];
}
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't know if I'm a huge fan of having this here..

Not a big deal probably... but this currently doesn't check if those paths already exist and would just overwrite them 🤷.

I feel like we should rework this function's API to be more like addTsConfigImportPaths(tree, importPath: string, importModules: string[])

function jsLibraryGenerator(tree) {
  addTsConfigPath(tree, importPath, [joinPathFragments(projectRoot, 'src/index.ts')])
  addTsConfigPath(tree, joinPathFragments(importPath, 'server'), [joinPathFragments(projectRoot, 'src/server.ts')])
}

This would open us up to adding import paths which are not under src.

We could add the new function and keep the current usages the same.. then transition them over. What do you think?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah we can do something like that.

Copy link
Member Author

@jaysoo jaysoo Apr 12, 2023

Choose a reason for hiding this comment

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

Removed the original function updateRootTsConfig and added addTsConfigPath so each library generator can add the import paths that are needed. The original function wasn't named clearly to understand the intention, the new function is more specific.

For now, the new server.ts entry is only for Next.js apps, and we can look at adding something similar to @nrwl/react:lib later when it's more clear the direction of RSCs.

packages/next/src/generators/library/library.ts Outdated Show resolved Hide resolved
Copy link
Collaborator

@FrozenPandaz FrozenPandaz left a comment

Choose a reason for hiding this comment

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

Sweet!

@jaysoo jaysoo force-pushed the feat/nextjs-13-2-update branch from 99f6619 to 83818c3 Compare April 12, 2023 14:28
@jaysoo jaysoo force-pushed the feat/nextjs-13-2-update branch from 83818c3 to 4c34f67 Compare April 12, 2023 14:28
@FrozenPandaz FrozenPandaz merged commit 0578116 into nrwl:master Apr 12, 2023
@jaysoo jaysoo deleted the feat/nextjs-13-2-update branch April 12, 2023 15:07
@github-actions
Copy link

This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants