-
Notifications
You must be signed in to change notification settings - Fork 26.8k
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
Fixes #31240: Adding a recursive addPackagePath function in webpack-config #31264
Fixes #31240: Adding a recursive addPackagePath function in webpack-config #31264
Conversation
Hey @ijjk @timneutkens @huozhi , it would be great to have your review here. |
I was able to roll back to v12.0.3 to get this to work, but can confirm that > v12.0.4 do not work. |
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.
You are bringing up a good point. We might be a bit more smart when creating these topLevelFrameworkPaths.
Instead of just assuming the scheduler is a dependency of react-dom, we should read the dependencies from package.json
.
So how about re-writing the code to:
const topLevelFrameworkPaths = new Set()
for(const package of ["react", "react-dom", "next"]) {
addPackagePath(name, dir)
}
and in addPackagePath
recursively visiting the dependencies listed in package.json:
const packageJsonPath = require.resolve(`${name}/package.json`, {
paths: [relativeToPath],
})
const directory = path.join(packageJsonPath, '../')
topLevelFrameworkPaths.add(directory)
const dependencies = require(packageJsonPath).dependencies
if(dependencies) {
for(const name of Object.keys(dependencies)) {
addPackagePath(name, directory)
}
}
and adding a try-catch too...
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
@neeraj3029 Looks like tests are failing due to these errors: https://github.com/vercel/next.js/runs/4324606853?check_suite_focus=true
Testing locally, the underlying (swallowed) error is likely this:
The |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Is there any update regarding this PR? Preact is not usable at all right now and it'll. be awesome if you can restore functionality |
This comment was marked as outdated.
This comment was marked as outdated.
This comment has been minimized.
This comment has been minimized.
Stats from current PRDefault Build (Increase detected
|
vercel/next.js canary | neeraj3029/next.js neeraj3029try-catch-getPackagePath | Change | |
---|---|---|---|
buildDuration | 18s | 17.9s | -101ms |
buildDurationCached | 7s | 7s | |
nodeModulesSize | 359 MB | 359 MB |
Page Load Tests Overall decrease ⚠️
vercel/next.js canary | neeraj3029/next.js neeraj3029try-catch-getPackagePath | Change | |
---|---|---|---|
/ failed reqs | 0 | 0 | ✓ |
/ total time (seconds) | 3.799 | 3.763 | -0.04 |
/ avg req/sec | 657.99 | 664.28 | +6.29 |
/error-in-render failed reqs | 0 | 0 | ✓ |
/error-in-render total time (seconds) | 1.549 | 1.57 | |
/error-in-render avg req/sec | 1613.93 | 1592.45 |
Client Bundles (main, webpack, commons) Overall decrease ✓
vercel/next.js canary | neeraj3029/next.js neeraj3029try-catch-getPackagePath | Change | |
---|---|---|---|
450.HASH.js gzip | 179 B | 179 B | ✓ |
framework-HASH.js gzip | 42.2 kB | 42 kB | -195 B |
main-HASH.js gzip | 27.4 kB | 27.4 kB | ✓ |
webpack-HASH.js gzip | 1.44 kB | 1.44 kB | ✓ |
Overall change | 71.2 kB | 71 kB | -195 B |
Legacy Client Bundles (polyfills)
vercel/next.js canary | neeraj3029/next.js neeraj3029try-catch-getPackagePath | Change | |
---|---|---|---|
polyfills-HASH.js gzip | 31 kB | 31 kB | ✓ |
Overall change | 31 kB | 31 kB | ✓ |
Client Pages Overall increase ⚠️
vercel/next.js canary | neeraj3029/next.js neeraj3029try-catch-getPackagePath | Change | |
---|---|---|---|
_app-HASH.js gzip | 1.36 kB | 1.36 kB | ✓ |
_error-HASH.js gzip | 194 B | 194 B | ✓ |
amp-HASH.js gzip | 312 B | 312 B | ✓ |
css-HASH.js gzip | 326 B | 326 B | ✓ |
dynamic-HASH.js gzip | 2.37 kB | 2.57 kB | |
head-HASH.js gzip | 350 B | 350 B | ✓ |
hooks-HASH.js gzip | 919 B | 919 B | ✓ |
image-HASH.js gzip | 4.94 kB | 4.94 kB | ✓ |
index-HASH.js gzip | 263 B | 263 B | ✓ |
link-HASH.js gzip | 2.2 kB | 2.2 kB | ✓ |
routerDirect..HASH.js gzip | 321 B | 321 B | ✓ |
script-HASH.js gzip | 383 B | 383 B | ✓ |
withRouter-HASH.js gzip | 318 B | 318 B | ✓ |
85e02e95b279..7e3.css gzip | 107 B | 107 B | ✓ |
Overall change | 14.4 kB | 14.6 kB |
Client Build Manifests
vercel/next.js canary | neeraj3029/next.js neeraj3029try-catch-getPackagePath | Change | |
---|---|---|---|
_buildManifest.js gzip | 460 B | 460 B | ✓ |
Overall change | 460 B | 460 B | ✓ |
Rendered Page Sizes Overall decrease ✓
vercel/next.js canary | neeraj3029/next.js neeraj3029try-catch-getPackagePath | Change | |
---|---|---|---|
index.html gzip | 532 B | 531 B | -1 B |
link.html gzip | 546 B | 545 B | -1 B |
withRouter.html gzip | 526 B | 526 B | ✓ |
Overall change | 1.6 kB | 1.6 kB | -2 B |
Diffs
Diff for _buildManifest.js
@@ -8,7 +8,7 @@ self.__BUILD_MANIFEST = {
"static\u002Fchunks\u002Fpages\u002Fcss-97182c5b8324021a.js"
],
"/dynamic": [
- "static\u002Fchunks\u002Fpages\u002Fdynamic-9b7afaaa04b653ed.js"
+ "static\u002Fchunks\u002Fpages\u002Fdynamic-b49fdb7088c2e39d.js"
],
"/head": ["static\u002Fchunks\u002Fpages\u002Fhead-7100d3b2a548f0e4.js"],
"/hooks": ["static\u002Fchunks\u002Fpages\u002Fhooks-538d621a0e670391.js"],
Diff for dynamic-HASH.js
@@ -633,6 +633,79 @@
) {
module.exports = __webpack_require__(638);
+ /***/
+ },
+
+ /***/ 8217: /***/ function(
+ __unused_webpack_module,
+ exports,
+ __webpack_require__
+ ) {
+ "use strict";
+ /** @license React vundefined
+ * use-subscription.production.min.js
+ *
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+ var e = __webpack_require__(6086),
+ g = __webpack_require__(7294);
+ exports.useSubscription = function(a) {
+ var c = a.getCurrentValue,
+ d = a.subscribe,
+ b = g.useState(function() {
+ return { getCurrentValue: c, subscribe: d, value: c() };
+ });
+ a = b[0];
+ var f = b[1];
+ b = a.value;
+ if (a.getCurrentValue !== c || a.subscribe !== d)
+ (b = c()), f({ getCurrentValue: c, subscribe: d, value: b });
+ g.useDebugValue(b);
+ g.useEffect(
+ function() {
+ function b() {
+ if (!a) {
+ var b = c();
+ f(function(a) {
+ return a.getCurrentValue !== c ||
+ a.subscribe !== d ||
+ a.value === b
+ ? a
+ : e({}, a, { value: b });
+ });
+ }
+ }
+ var a = !1,
+ h = d(b);
+ b();
+ return function() {
+ a = !0;
+ h();
+ };
+ },
+ [c, d]
+ );
+ return b;
+ };
+
+ /***/
+ },
+
+ /***/ 7161: /***/ function(
+ module,
+ __unused_webpack_exports,
+ __webpack_require__
+ ) {
+ "use strict";
+
+ if (true) {
+ module.exports = __webpack_require__(8217);
+ } else {
+ }
+
/***/
}
},
Diff for framework-HASH.js
@@ -8688,76 +8688,6 @@
} else {
}
- /***/
- },
-
- /***/ 8217: /***/ function(
- __unused_webpack_module,
- exports,
- __webpack_require__
- ) {
- /** @license React vundefined
- * use-subscription.production.min.js
- *
- * Copyright (c) Facebook, Inc. and its affiliates.
- *
- * This source code is licensed under the MIT license found in the
- * LICENSE file in the root directory of this source tree.
- */
- var e = __webpack_require__(6086),
- g = __webpack_require__(7294);
- exports.useSubscription = function(a) {
- var c = a.getCurrentValue,
- d = a.subscribe,
- b = g.useState(function() {
- return { getCurrentValue: c, subscribe: d, value: c() };
- });
- a = b[0];
- var f = b[1];
- b = a.value;
- if (a.getCurrentValue !== c || a.subscribe !== d)
- (b = c()), f({ getCurrentValue: c, subscribe: d, value: b });
- g.useDebugValue(b);
- g.useEffect(
- function() {
- function b() {
- if (!a) {
- var b = c();
- f(function(a) {
- return a.getCurrentValue !== c ||
- a.subscribe !== d ||
- a.value === b
- ? a
- : e({}, a, { value: b });
- });
- }
- }
- var a = !1,
- h = d(b);
- b();
- return function() {
- a = !0;
- h();
- };
- },
- [c, d]
- );
- return b;
- };
-
- /***/
- },
-
- /***/ 7161: /***/ function(
- module,
- __unused_webpack_exports,
- __webpack_require__
- ) {
- if (true) {
- module.exports = __webpack_require__(8217);
- } else {
- }
-
/***/
}
}
Diff for index.html
@@ -15,7 +15,7 @@
defer=""
></script>
<script
- src="/_next/static/chunks/framework-535325b9235a831f.js"
+ src="/_next/static/chunks/framework-287ad9396a03b869.js"
defer=""
></script>
<script
Diff for link.html
@@ -15,7 +15,7 @@
defer=""
></script>
<script
- src="/_next/static/chunks/framework-535325b9235a831f.js"
+ src="/_next/static/chunks/framework-287ad9396a03b869.js"
defer=""
></script>
<script
Diff for withRouter.html
@@ -15,7 +15,7 @@
defer=""
></script>
<script
- src="/_next/static/chunks/framework-535325b9235a831f.js"
+ src="/_next/static/chunks/framework-287ad9396a03b869.js"
defer=""
></script>
<script
Default Build with SWC (Increase detected ⚠️ )
General Overall increase ⚠️
vercel/next.js canary | neeraj3029/next.js neeraj3029try-catch-getPackagePath | Change | |
---|---|---|---|
buildDuration | 21.9s | 21.7s | -204ms |
buildDurationCached | 7s | 7.2s | |
nodeModulesSize | 359 MB | 359 MB |
Page Load Tests Overall increase ✓
vercel/next.js canary | neeraj3029/next.js neeraj3029try-catch-getPackagePath | Change | |
---|---|---|---|
/ failed reqs | 0 | 0 | ✓ |
/ total time (seconds) | 3.767 | 3.712 | -0.05 |
/ avg req/sec | 663.61 | 673.47 | +9.86 |
/error-in-render failed reqs | 0 | 0 | ✓ |
/error-in-render total time (seconds) | 1.58 | 1.547 | -0.03 |
/error-in-render avg req/sec | 1582.62 | 1616.07 | +33.45 |
Client Bundles (main, webpack, commons) Overall decrease ✓
vercel/next.js canary | neeraj3029/next.js neeraj3029try-catch-getPackagePath | Change | |
---|---|---|---|
450.HASH.js gzip | 179 B | 179 B | ✓ |
framework-HASH.js gzip | 42.3 kB | 42.1 kB | -190 B |
main-HASH.js gzip | 27.4 kB | 27.4 kB | ✓ |
webpack-HASH.js gzip | 1.44 kB | 1.44 kB | ✓ |
Overall change | 71.3 kB | 71.2 kB | -190 B |
Legacy Client Bundles (polyfills)
vercel/next.js canary | neeraj3029/next.js neeraj3029try-catch-getPackagePath | Change | |
---|---|---|---|
polyfills-HASH.js gzip | 31 kB | 31 kB | ✓ |
Overall change | 31 kB | 31 kB | ✓ |
Client Pages Overall increase ⚠️
vercel/next.js canary | neeraj3029/next.js neeraj3029try-catch-getPackagePath | Change | |
---|---|---|---|
_app-HASH.js gzip | 1.35 kB | 1.35 kB | ✓ |
_error-HASH.js gzip | 180 B | 180 B | ✓ |
amp-HASH.js gzip | 305 B | 305 B | ✓ |
css-HASH.js gzip | 321 B | 321 B | ✓ |
dynamic-HASH.js gzip | 2.36 kB | 2.56 kB | |
head-HASH.js gzip | 342 B | 342 B | ✓ |
hooks-HASH.js gzip | 911 B | 911 B | ✓ |
image-HASH.js gzip | 4.97 kB | 4.97 kB | ✓ |
index-HASH.js gzip | 256 B | 256 B | ✓ |
link-HASH.js gzip | 2.22 kB | 2.22 kB | ✓ |
routerDirect..HASH.js gzip | 314 B | 314 B | ✓ |
script-HASH.js gzip | 375 B | 375 B | ✓ |
withRouter-HASH.js gzip | 309 B | 309 B | ✓ |
85e02e95b279..7e3.css gzip | 107 B | 107 B | ✓ |
Overall change | 14.3 kB | 14.5 kB |
Client Build Manifests
vercel/next.js canary | neeraj3029/next.js neeraj3029try-catch-getPackagePath | Change | |
---|---|---|---|
_buildManifest.js gzip | 459 B | 459 B | ✓ |
Overall change | 459 B | 459 B | ✓ |
Rendered Page Sizes Overall increase ⚠️
vercel/next.js canary | neeraj3029/next.js neeraj3029try-catch-getPackagePath | Change | |
---|---|---|---|
index.html gzip | 533 B | 534 B | |
link.html gzip | 546 B | 547 B | |
withRouter.html gzip | 528 B | 528 B | ✓ |
Overall change | 1.61 kB | 1.61 kB |
Diffs
Diff for _buildManifest.js
@@ -8,7 +8,7 @@ self.__BUILD_MANIFEST = {
"static\u002Fchunks\u002Fpages\u002Fcss-97182c5b8324021a.js"
],
"/dynamic": [
- "static\u002Fchunks\u002Fpages\u002Fdynamic-9b7afaaa04b653ed.js"
+ "static\u002Fchunks\u002Fpages\u002Fdynamic-b49fdb7088c2e39d.js"
],
"/head": ["static\u002Fchunks\u002Fpages\u002Fhead-7100d3b2a548f0e4.js"],
"/hooks": ["static\u002Fchunks\u002Fpages\u002Fhooks-538d621a0e670391.js"],
Diff for dynamic-HASH.js
@@ -633,6 +633,79 @@
) {
module.exports = __webpack_require__(638);
+ /***/
+ },
+
+ /***/ 8217: /***/ function(
+ __unused_webpack_module,
+ exports,
+ __webpack_require__
+ ) {
+ "use strict";
+ /** @license React vundefined
+ * use-subscription.production.min.js
+ *
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+ var e = __webpack_require__(6086),
+ g = __webpack_require__(7294);
+ exports.useSubscription = function(a) {
+ var c = a.getCurrentValue,
+ d = a.subscribe,
+ b = g.useState(function() {
+ return { getCurrentValue: c, subscribe: d, value: c() };
+ });
+ a = b[0];
+ var f = b[1];
+ b = a.value;
+ if (a.getCurrentValue !== c || a.subscribe !== d)
+ (b = c()), f({ getCurrentValue: c, subscribe: d, value: b });
+ g.useDebugValue(b);
+ g.useEffect(
+ function() {
+ function b() {
+ if (!a) {
+ var b = c();
+ f(function(a) {
+ return a.getCurrentValue !== c ||
+ a.subscribe !== d ||
+ a.value === b
+ ? a
+ : e({}, a, { value: b });
+ });
+ }
+ }
+ var a = !1,
+ h = d(b);
+ b();
+ return function() {
+ a = !0;
+ h();
+ };
+ },
+ [c, d]
+ );
+ return b;
+ };
+
+ /***/
+ },
+
+ /***/ 7161: /***/ function(
+ module,
+ __unused_webpack_exports,
+ __webpack_require__
+ ) {
+ "use strict";
+
+ if (true) {
+ module.exports = __webpack_require__(8217);
+ } else {
+ }
+
/***/
}
},
Diff for framework-HASH.js
@@ -8688,76 +8688,6 @@
} else {
}
- /***/
- },
-
- /***/ 8217: /***/ function(
- __unused_webpack_module,
- exports,
- __webpack_require__
- ) {
- /** @license React vundefined
- * use-subscription.production.min.js
- *
- * Copyright (c) Facebook, Inc. and its affiliates.
- *
- * This source code is licensed under the MIT license found in the
- * LICENSE file in the root directory of this source tree.
- */
- var e = __webpack_require__(6086),
- g = __webpack_require__(7294);
- exports.useSubscription = function(a) {
- var c = a.getCurrentValue,
- d = a.subscribe,
- b = g.useState(function() {
- return { getCurrentValue: c, subscribe: d, value: c() };
- });
- a = b[0];
- var f = b[1];
- b = a.value;
- if (a.getCurrentValue !== c || a.subscribe !== d)
- (b = c()), f({ getCurrentValue: c, subscribe: d, value: b });
- g.useDebugValue(b);
- g.useEffect(
- function() {
- function b() {
- if (!a) {
- var b = c();
- f(function(a) {
- return a.getCurrentValue !== c ||
- a.subscribe !== d ||
- a.value === b
- ? a
- : e({}, a, { value: b });
- });
- }
- }
- var a = !1,
- h = d(b);
- b();
- return function() {
- a = !0;
- h();
- };
- },
- [c, d]
- );
- return b;
- };
-
- /***/
- },
-
- /***/ 7161: /***/ function(
- module,
- __unused_webpack_exports,
- __webpack_require__
- ) {
- if (true) {
- module.exports = __webpack_require__(8217);
- } else {
- }
-
/***/
}
}
Diff for index.html
@@ -15,7 +15,7 @@
defer=""
></script>
<script
- src="/_next/static/chunks/framework-535325b9235a831f.js"
+ src="/_next/static/chunks/framework-287ad9396a03b869.js"
defer=""
></script>
<script
Diff for link.html
@@ -15,7 +15,7 @@
defer=""
></script>
<script
- src="/_next/static/chunks/framework-535325b9235a831f.js"
+ src="/_next/static/chunks/framework-287ad9396a03b869.js"
defer=""
></script>
<script
Diff for withRouter.html
@@ -15,7 +15,7 @@
defer=""
></script>
<script
- src="/_next/static/chunks/framework-535325b9235a831f.js"
+ src="/_next/static/chunks/framework-287ad9396a03b869.js"
defer=""
></script>
<script
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.
Thanks for the PR!
…pack-config (vercel#31264) Fixes: vercel#31240 Closes: vercel#32324 Adding a try-catch block to handle situations when packages are found at relative path in getPackagePath function. This is likely to occur when using `preact` instead of `react-dom`, as `scheduler` package will not be found wrt `react-dom` ## Bug - [x] Related issues linked using `fixes vercel#31240` Co-authored-by: JJ Kasper <22380829+ijjk@users.noreply.github.com>
Fixes: #31240
Closes: #32324
Adding a try-catch block to handle situations when packages are found at relative path in getPackagePath function. This is likely to occur when using
preact
instead ofreact-dom
, asscheduler
package will not be found wrtreact-dom
Bug
fixes #31240