Skip to content

Commit

Permalink
use postcss transforms consistently also on server side (#57434)
Browse files Browse the repository at this point in the history
### What?

Server side should also apply postcss transforms in node_modules

### Why?

### How?


Closes WEB-1863
  • Loading branch information
sokra authored Oct 26, 2023
1 parent 1df027f commit f7a7a8e
Show file tree
Hide file tree
Showing 7 changed files with 59 additions and 22 deletions.
11 changes: 8 additions & 3 deletions packages/next-swc/crates/napi/src/next_api/endpoint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ pub async fn endpoint_write_to_disk(
#[napi(ts_return_type = "{ __napiType: \"RootTask\" }")]
pub fn endpoint_server_changed_subscribe(
#[napi(ts_arg_type = "{ __napiType: \"Endpoint\" }")] endpoint: External<ExternalEndpoint>,
issues: bool,
func: JsFunction,
) -> napi::Result<External<RootTask>> {
let turbo_tasks = endpoint.turbo_tasks().clone();
Expand All @@ -124,9 +125,13 @@ pub fn endpoint_server_changed_subscribe(
move || async move {
let changed = endpoint.server_changed();
changed.strongly_consistent().await?;
let issues = get_issues(changed).await?;
let diags = get_diagnostics(changed).await?;
Ok((issues, diags))
if issues {
let issues = get_issues(changed).await?;
let diags = get_diagnostics(changed).await?;
Ok((issues, diags))
} else {
Ok((vec![], vec![]))
}
},
|ctx| {
let (issues, diags) = ctx.value;
Expand Down
10 changes: 7 additions & 3 deletions packages/next-swc/crates/next-api/src/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -812,22 +812,23 @@ impl Project {
#[turbo_tasks::function]
pub fn server_changed(self: Vc<Self>, roots: Vc<OutputAssets>) -> Vc<Completion> {
let path = self.node_root();
any_output_changed(roots, path)
any_output_changed(roots, path, true)
}

/// Completion when client side changes are detected in output assets
/// referenced from the roots
#[turbo_tasks::function]
pub fn client_changed(self: Vc<Self>, roots: Vc<OutputAssets>) -> Vc<Completion> {
let path = self.client_root();
any_output_changed(roots, path)
any_output_changed(roots, path, false)
}
}

#[turbo_tasks::function]
async fn any_output_changed(
roots: Vc<OutputAssets>,
path: Vc<FileSystemPath>,
server: bool,
) -> Result<Vc<Completion>> {
let path = &path.await?;
let completions = AdjacencyMap::new()
Expand All @@ -839,7 +840,10 @@ async fn any_output_changed(
.into_reverse_topological()
.map(|m| async move {
let asset_path = m.ident().path().await?;
if !asset_path.path.ends_with(".map") && asset_path.is_inside_ref(path) {
if !asset_path.path.ends_with(".map")
&& (!server || !asset_path.path.ends_with(".css"))
&& asset_path.is_inside_ref(path)
{
Ok(Some(content_changed(Vc::upcast(m))))
} else {
Ok(None)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -279,8 +279,8 @@ pub async fn get_client_module_options_context(
// we try resolve it once at the root and pass down a context to all
// the modules.
enable_jsx: Some(jsx_runtime_options),
enable_postcss_transform: postcss_transform_options,
enable_webpack_loaders,
enable_postcss_transform: postcss_transform_options,
enable_typescript_transform: Some(tsconfig),
enable_mdx_rs,
decorators: Some(decorators_options),
Expand Down
18 changes: 12 additions & 6 deletions packages/next-swc/crates/next-core/src/next_server/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ pub async fn get_server_module_options_context(

let foreign_code_context_condition =
foreign_code_context_condition(next_config, project_path).await?;
let enable_postcss_transform = Some(PostCssTransformOptions {
let postcss_transform_options = Some(PostCssTransformOptions {
postcss_package: Some(get_postcss_package_mapping(project_path)),
..Default::default()
});
Expand Down Expand Up @@ -351,6 +351,8 @@ pub async fn get_server_module_options_context(
let foreign_code_module_options_context = ModuleOptionsContext {
custom_rules: internal_custom_rules.clone(),
enable_webpack_loaders: foreign_webpack_loaders,
// NOTE(WEB-1016) PostCSS transforms should also apply to foreign code.
enable_postcss_transform: postcss_transform_options.clone(),
..module_options_context.clone()
};

Expand All @@ -363,8 +365,8 @@ pub async fn get_server_module_options_context(

ModuleOptionsContext {
enable_jsx: Some(jsx_runtime_options),
enable_postcss_transform,
enable_webpack_loaders,
enable_postcss_transform: postcss_transform_options,
enable_typescript_transform: Some(tsconfig),
enable_mdx_rs,
decorators: Some(decorators_options),
Expand Down Expand Up @@ -416,6 +418,8 @@ pub async fn get_server_module_options_context(
let foreign_code_module_options_context = ModuleOptionsContext {
custom_rules: internal_custom_rules.clone(),
enable_webpack_loaders: foreign_webpack_loaders,
// NOTE(WEB-1016) PostCSS transforms should also apply to foreign code.
enable_postcss_transform: postcss_transform_options.clone(),
..module_options_context.clone()
};
let internal_module_options_context = ModuleOptionsContext {
Expand All @@ -436,8 +440,8 @@ pub async fn get_server_module_options_context(

ModuleOptionsContext {
enable_jsx: Some(jsx_runtime_options),
enable_postcss_transform,
enable_webpack_loaders,
enable_postcss_transform: postcss_transform_options,
enable_typescript_transform: Some(tsconfig),
enable_mdx_rs,
decorators: Some(decorators_options),
Expand Down Expand Up @@ -498,6 +502,8 @@ pub async fn get_server_module_options_context(
let foreign_code_module_options_context = ModuleOptionsContext {
custom_rules: internal_custom_rules.clone(),
enable_webpack_loaders: foreign_webpack_loaders,
// NOTE(WEB-1016) PostCSS transforms should also apply to foreign code.
enable_postcss_transform: postcss_transform_options.clone(),
..module_options_context.clone()
};
let internal_module_options_context = ModuleOptionsContext {
Expand All @@ -507,8 +513,8 @@ pub async fn get_server_module_options_context(
};
ModuleOptionsContext {
enable_jsx: Some(jsx_runtime_options),
enable_postcss_transform,
enable_webpack_loaders,
enable_postcss_transform: postcss_transform_options,
enable_typescript_transform: Some(tsconfig),
enable_mdx_rs,
decorators: Some(decorators_options),
Expand Down Expand Up @@ -538,8 +544,8 @@ pub async fn get_server_module_options_context(
..module_options_context.clone()
};
ModuleOptionsContext {
enable_postcss_transform,
enable_webpack_loaders,
enable_postcss_transform: postcss_transform_options,
enable_typescript_transform: Some(tsconfig),
enable_mdx_rs,
decorators: Some(decorators_options),
Expand Down Expand Up @@ -587,8 +593,8 @@ pub async fn get_server_module_options_context(
};
ModuleOptionsContext {
enable_jsx: Some(jsx_runtime_options),
enable_postcss_transform,
enable_webpack_loaders,
enable_postcss_transform: postcss_transform_options,
enable_typescript_transform: Some(tsconfig),
enable_mdx_rs,
decorators: Some(decorators_options),
Expand Down
9 changes: 7 additions & 2 deletions packages/next/src/build/swc/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -585,7 +585,9 @@ export interface Endpoint {
* After serverChanged() has been awaited it will listen to changes.
* The async iterator will yield for each change.
*/
serverChanged(): Promise<AsyncIterableIterator<TurbopackResult>>
serverChanged(
includeIssues: boolean
): Promise<AsyncIterableIterator<TurbopackResult>>
}

interface EndpointConfig {
Expand Down Expand Up @@ -947,12 +949,15 @@ function bindingToApi(binding: any, _wasm: boolean) {
return clientSubscription
}

async serverChanged(): Promise<AsyncIterableIterator<TurbopackResult<{}>>> {
async serverChanged(
includeIssues: boolean
): Promise<AsyncIterableIterator<TurbopackResult<{}>>> {
const serverSubscription = subscribe<TurbopackResult>(
false,
async (callback) =>
binding.endpointServerChangedSubscribe(
await this._nativeEndpoint,
includeIssues,
callback
)
)
Expand Down
23 changes: 18 additions & 5 deletions packages/next/src/server/lib/router-utils/setup-dev-bundler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -634,6 +634,7 @@ async function startWatcher(opts: SetupOpts) {
async function changeSubscription(
page: string,
type: 'client' | 'server',
includeIssues: boolean,
endpoint: Endpoint | undefined,
makePayload: (
page: string,
Expand All @@ -643,7 +644,7 @@ async function startWatcher(opts: SetupOpts) {
const key = `${page} (${type})`
if (!endpoint || changeSubscriptions.has(key)) return

const changedPromise = endpoint[`${type}Changed`]()
const changedPromise = endpoint[`${type}Changed`](includeIssues)
changeSubscriptions.set(key, changedPromise)
const changed = await changedPromise

Expand Down Expand Up @@ -1105,6 +1106,7 @@ async function startWatcher(opts: SetupOpts) {
changeSubscription(
'middleware',
'server',
false,
middleware.endpoint,
async () => {
const finishBuilding = startBuilding('middleware', true)
Expand Down Expand Up @@ -1327,6 +1329,7 @@ async function startWatcher(opts: SetupOpts) {
changeSubscription(
'_document',
'server',
false,
globalEntries.document,
() => {
return { action: HMR_ACTIONS_SENT_TO_BROWSER.RELOAD_PAGE }
Expand Down Expand Up @@ -1425,6 +1428,7 @@ async function startWatcher(opts: SetupOpts) {
changeSubscription(
'_document',
'server',
false,
globalEntries.document,
() => {
return { action: HMR_ACTIONS_SENT_TO_BROWSER.RELOAD_PAGE }
Expand Down Expand Up @@ -1461,19 +1465,27 @@ async function startWatcher(opts: SetupOpts) {
changeSubscription(
page,
'server',
false,
route.dataEndpoint,
(pageName) => {
console.log('server change', pageName)
return {
event: HMR_ACTIONS_SENT_TO_BROWSER.SERVER_ONLY_CHANGES,
pages: [pageName],
}
}
)
changeSubscription(page, 'client', route.htmlEndpoint, () => {
return {
event: HMR_ACTIONS_SENT_TO_BROWSER.CLIENT_CHANGES,
changeSubscription(
page,
'client',
false,
route.htmlEndpoint,
() => {
return {
event: HMR_ACTIONS_SENT_TO_BROWSER.CLIENT_CHANGES,
}
}
})
)
}

break
Expand Down Expand Up @@ -1517,6 +1529,7 @@ async function startWatcher(opts: SetupOpts) {
changeSubscription(
page,
'server',
true,
route.rscEndpoint,
(_page, change) => {
if (
Expand Down
8 changes: 6 additions & 2 deletions test/development/basic/next-rs-api.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -439,12 +439,16 @@ describe('next.rs api', () => {
switch (route.type) {
case 'page': {
await route.htmlEndpoint.writeToDisk()
serverSideSubscription = await route.dataEndpoint.serverChanged()
serverSideSubscription = await route.dataEndpoint.serverChanged(
false
)
break
}
case 'app-page': {
await route.htmlEndpoint.writeToDisk()
serverSideSubscription = await route.rscEndpoint.serverChanged()
serverSideSubscription = await route.rscEndpoint.serverChanged(
false
)
break
}
default: {
Expand Down

1 comment on commit f7a7a8e

@ijjk
Copy link
Member

@ijjk ijjk commented on f7a7a8e Oct 26, 2023

Choose a reason for hiding this comment

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

Stats from current release

Default Build (Increase detected ⚠️)
General Overall increase ⚠️
vercel/next.js canary v13.5.6 vercel/next.js canary Change
buildDuration 10.5s 11.1s ⚠️ +641ms
buildDurationCached 6.2s 6s N/A
nodeModulesSize 172 MB 175 MB ⚠️ +2.59 MB
nextStartRea..uration (ms) 511ms 399ms N/A
Client Bundles (main, webpack) Overall increase ⚠️
vercel/next.js canary v13.5.6 vercel/next.js canary Change
199-HASH.js gzip 27.5 kB N/A N/A
3f784ff6-HASH.js gzip 50.9 kB N/A N/A
99.HASH.js gzip 182 B 182 B
framework-HASH.js gzip 45.3 kB 45.5 kB ⚠️ +223 B
main-app-HASH.js gzip 254 B 251 B N/A
main-HASH.js gzip 32.9 kB 35.4 kB ⚠️ +2.49 kB
webpack-HASH.js gzip 1.75 kB 1.75 kB N/A
3c4a14c2-HASH.js gzip N/A 53.2 kB N/A
513-HASH.js gzip N/A 32.4 kB N/A
Overall change 78.4 kB 81.1 kB ⚠️ +2.71 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary v13.5.6 vercel/next.js canary Change
polyfills-HASH.js gzip 31 kB 31 kB
Overall change 31 kB 31 kB
Client Pages
vercel/next.js canary v13.5.6 vercel/next.js canary Change
_app-HASH.js gzip 206 B 205 B N/A
_error-HASH.js gzip 182 B 180 B N/A
amp-HASH.js gzip 506 B 505 B N/A
css-HASH.js gzip 322 B 323 B N/A
dynamic-HASH.js gzip 2.57 kB 2.59 kB N/A
edge-ssr-HASH.js gzip 260 B 259 B N/A
head-HASH.js gzip 350 B 350 B
hooks-HASH.js gzip 369 B 369 B
image-HASH.js gzip 4.35 kB 4.38 kB N/A
index-HASH.js gzip 256 B 256 B
link-HASH.js gzip 2.64 kB 2.67 kB N/A
routerDirect..HASH.js gzip 312 B 318 B N/A
script-HASH.js gzip 385 B 384 B N/A
withRouter-HASH.js gzip 307 B 319 B N/A
1afbb74e6ecf..834.css gzip 106 B 106 B
Overall change 1.08 kB 1.08 kB
Client Build Manifests
vercel/next.js canary v13.5.6 vercel/next.js canary Change
_buildManifest.js gzip 485 B 482 B N/A
Overall change 0 B 0 B
Rendered Page Sizes
vercel/next.js canary v13.5.6 vercel/next.js canary Change
index.html gzip 527 B 529 B N/A
link.html gzip 541 B 543 B N/A
withRouter.html gzip 524 B 524 B
Overall change 524 B 524 B
Edge SSR bundle Size Overall increase ⚠️
vercel/next.js canary v13.5.6 vercel/next.js canary Change
edge-ssr.js gzip 93.9 kB 96 kB ⚠️ +2.1 kB
page.js gzip 152 kB 140 kB N/A
Overall change 93.9 kB 96 kB ⚠️ +2.1 kB
Middleware size Overall increase ⚠️
vercel/next.js canary v13.5.6 vercel/next.js canary Change
middleware-b..fest.js gzip 624 B 624 B
middleware-r..fest.js gzip 150 B 151 B N/A
middleware.js gzip 22.9 kB 25.6 kB ⚠️ +2.69 kB
edge-runtime..pack.js gzip 1.92 kB 1.92 kB
Overall change 25.5 kB 28.1 kB ⚠️ +2.69 kB
Diff details
Diff for page.js
failed to diff
Diff for middleware.js

Diff too large to display

Diff for edge-ssr.js

Diff too large to display

Diff for image-HASH.js
@@ -1,7 +1,7 @@
 (self["webpackChunk_N_E"] = self["webpackChunk_N_E"] || []).push([
   [358],
   {
-    /***/ 1552: /***/ function (
+    /***/ 4070: /***/ function (
       __unused_webpack_module,
       __unused_webpack_exports,
       __webpack_require__
@@ -9,7 +9,7 @@
       (window.__NEXT_P = window.__NEXT_P || []).push([
         "/image",
         function () {
-          return __webpack_require__(528);
+          return __webpack_require__(5921);
         },
       ]);
       if (false) {
@@ -18,7 +18,7 @@
       /***/
     },
 
-    /***/ 4036: /***/ function (module, exports, __webpack_require__) {
+    /***/ 2155: /***/ function (module, exports, __webpack_require__) {
       "use strict";
       /* __next_internal_client_entry_do_not_use__  cjs */
       Object.defineProperty(exports, "__esModule", {
@@ -39,15 +39,15 @@
         __webpack_require__(422)
       );
       const _head = /*#__PURE__*/ _interop_require_default._(
-        __webpack_require__(8777)
+        __webpack_require__(9561)
       );
-      const _getimgprops = __webpack_require__(2391);
-      const _imageconfig = __webpack_require__(1138);
-      const _imageconfigcontextsharedruntime = __webpack_require__(5615);
-      const _warnonce = __webpack_require__(7536);
-      const _routercontextsharedruntime = __webpack_require__(6174);
+      const _getimgprops = __webpack_require__(3787);
+      const _imageconfig = __webpack_require__(3767);
+      const _imageconfigcontextsharedruntime = __webpack_require__(8744);
+      const _warnonce = __webpack_require__(6908);
+      const _routercontextsharedruntime = __webpack_require__(937);
       const _imageloader = /*#__PURE__*/ _interop_require_default._(
-        __webpack_require__(7512)
+        __webpack_require__(2534)
       );
       // This is replaced by webpack define plugin
       const configEnv = {
@@ -127,7 +127,7 @@
         });
       }
       function getDynamicProps(fetchPriority) {
-        const [majorStr, minorStr] = _react.version.split(".");
+        const [majorStr, minorStr] = _react.version.split(".", 2);
         const major = parseInt(majorStr, 10);
         const minor = parseInt(minorStr, 10);
         if (major > 18 || (major === 18 && minor >= 3)) {
@@ -372,7 +372,7 @@
       /***/
     },
 
-    /***/ 2391: /***/ function (
+    /***/ 3787: /***/ function (
       __unused_webpack_module,
       exports,
       __webpack_require__
@@ -388,9 +388,9 @@
           return getImgProps;
         },
       });
-      const _warnonce = __webpack_require__(7536);
-      const _imageblursvg = __webpack_require__(9080);
-      const _imageconfig = __webpack_require__(1138);
+      const _warnonce = __webpack_require__(6908);
+      const _imageblursvg = __webpack_require__(9317);
+      const _imageconfig = __webpack_require__(3767);
       const VALID_LOADING_VALUES =
         /* unused pure expression or super */ null && [
           "lazy",
@@ -759,7 +759,7 @@
       /***/
     },
 
-    /***/ 9080: /***/ function (__unused_webpack_module, exports) {
+    /***/ 9317: /***/ function (__unused_webpack_module, exports) {
       "use strict";
       /**
        * A shared function, used on both client and server, to generate a SVG blur placeholder.
@@ -814,7 +814,7 @@
       /***/
     },
 
-    /***/ 33: /***/ function (
+    /***/ 7196: /***/ function (
       __unused_webpack_module,
       exports,
       __webpack_require__
@@ -841,11 +841,11 @@
         },
       });
       const _interop_require_default = __webpack_require__(1351);
-      const _getimgprops = __webpack_require__(2391);
-      const _warnonce = __webpack_require__(7536);
-      const _imagecomponent = __webpack_require__(4036);
+      const _getimgprops = __webpack_require__(3787);
+      const _warnonce = __webpack_require__(6908);
+      const _imagecomponent = __webpack_require__(2155);
       const _imageloader = /*#__PURE__*/ _interop_require_default._(
-        __webpack_require__(7512)
+        __webpack_require__(2534)
       );
       const unstable_getImgProps = (imgProps) => {
         (0, _warnonce.warnOnce)(
@@ -877,7 +877,7 @@
       /***/
     },
 
-    /***/ 7512: /***/ function (__unused_webpack_module, exports) {
+    /***/ 2534: /***/ function (__unused_webpack_module, exports) {
       "use strict";
 
       Object.defineProperty(exports, "__esModule", {
@@ -912,7 +912,7 @@
       /***/
     },
 
-    /***/ 528: /***/ function (
+    /***/ 5921: /***/ function (
       __unused_webpack_module,
       __webpack_exports__,
       __webpack_require__
@@ -933,8 +933,8 @@
 
       // EXTERNAL MODULE: ./node_modules/.pnpm/react@18.2.0/node_modules/react/jsx-runtime.js
       var jsx_runtime = __webpack_require__(1527);
-      // EXTERNAL MODULE: ./node_modules/.pnpm/file+..+main-repo+packages+next+next-packed.tgz_react-dom@18.2.0_react@18.2.0/node_modules/next/image.js
-      var next_image = __webpack_require__(1577);
+      // EXTERNAL MODULE: ./node_modules/.pnpm/file+..+diff-repo+packages+next+next-packed.tgz_react-dom@18.2.0_react@18.2.0/node_modules/next/image.js
+      var next_image = __webpack_require__(73);
       var image_default = /*#__PURE__*/ __webpack_require__.n(next_image); // CONCATENATED MODULE: ./pages/nextjs.png
       /* harmony default export */ var nextjs = {
         src: "/_next/static/media/nextjs.cae0b805.png",
@@ -964,12 +964,12 @@
       /***/
     },
 
-    /***/ 1577: /***/ function (
+    /***/ 73: /***/ function (
       module,
       __unused_webpack_exports,
       __webpack_require__
     ) {
-      module.exports = __webpack_require__(33);
+      module.exports = __webpack_require__(7196);
 
       /***/
     },
@@ -980,7 +980,7 @@
       return __webpack_require__((__webpack_require__.s = moduleId));
     };
     /******/ __webpack_require__.O(0, [774, 888, 179], function () {
-      return __webpack_exec__(1552);
+      return __webpack_exec__(4070);
     });
     /******/ var __webpack_exports__ = __webpack_require__.O();
     /******/ _N_E = __webpack_exports__;
Diff for 199-HASH.js

Diff too large to display

Diff for 3f784ff6-HASH.js

Diff too large to display

Diff for main-HASH.js

Diff too large to display

Please sign in to comment.