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.

Sign up for GitHub

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

Jump to bottom

Build: prefer root node_modules to reduce duplication during bundling #32578

Closed
wants to merge 1 commit into from
Closed

Build: prefer root node_modules to reduce duplication during bundling #32578

wants to merge 1 commit into from

Conversation

leeoniya
Copy link
Contributor

@leeoniya leeoniya commented Apr 1, 2021

this eliminates the entire (duplicated) packages section on the right:

image

@leeoniya leeoniya removed the request for review from a team April 1, 2021 03:42
hugohaggmark
hugohaggmark reviewed Apr 1, 2021
View reviewed changes
package.json
@@ -269,6 +269,7 @@
"re-resizable": "^6.2.0",
"react": "17.0.1",
"react-beautiful-dnd": "13.0.0",
"react-color": "2.18.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this a mistake? I can't see a corresponding yarn.lock file change.

Copy link
Contributor Author

@leeoniya leeoniya Apr 1, 2021 •
edited

Choose a reason for hiding this comment

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

this is on purpose. this was already required inside grafana-ui, but i also required it at root so that when it gets bundled it's within vendors/node_modules like everything else, instead of in vendors/packages/grafana-ui/node_modules. it was the only dep that was remaining in that bucket, so this change moves it into the same location as the rest, otherwise it's a thin squished line that has no labels until you zoom in a ton.

the implication being, that it was the only non-duplicate import in that group :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, thanks for clarification.

hugohaggmark
hugohaggmark previously requested changes Apr 1, 2021
View reviewed changes
Copy link
Contributor

@hugohaggmark hugohaggmark left a comment

Choose a reason for hiding this comment

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

Looks good and it works as expected when I test with both linked and unlinked Enterprise repo.

But, not sure how this is related to the changes on this branch, but I get this error when I try to navigate to Explore.

Steps to repro

  1. rm -rf ./node_modules/
  2. make run-frontend
  3. http://localhost:3000/explore

image

Uncaught TypeError: Object(...) is not a function
    at Trigger.render (index.js:685)
    at finishClassComponent (react-dom.development.js:17485)
    at updateClassComponent (react-dom.development.js:17435)
    at beginWork (react-dom.development.js:19073)
    at HTMLUnknownElement.callCallback (react-dom.development.js:3945)
    at Object.invokeGuardedCallbackDev (react-dom.development.js:3994)
    at invokeGuardedCallback (react-dom.development.js:4056)
    at beginWork$1 (react-dom.development.js:23964)
    at performUnitOfWork (react-dom.development.js:22776)
    at workLoopSync (react-dom.development.js:22707)
react-dom.development.js:20085 The above error occurred in the <Trigger> component:

    at Trigger (eval at ES6ProxyComponentFactory (http://localhost:3000/public/build/vendors~app.86e74a86d4afc6733e22.js:262445:10), <anonymous>:5:7)
    at Cascader (http://localhost:3000/public/build/vendors~app.86e74a86d4afc6733e22.js:202501:5)
    at ButtonCascader (http://localhost:3000/public/build/default~app.86e74a86d4afc6733e22.js:34352:24)
    at div
    at div
    at PromQueryField (http://localhost:3000/public/build/prometheusPlugin.86e74a86d4afc6733e22.js:969:5)
    at PromExploreQueryEditor (http://localhost:3000/public/build/prometheusPlugin.86e74a86d4afc6733e22.js:310:21)
    at div
    at div
    at QueryRow (eval at ES6ProxyComponentFactory (http://localhost:3000/public/build/vendors~app.86e74a86d4afc6733e22.js:262445:10), <anonymous>:5:7)
    at Connect(QueryRow) (http://localhost:3000/public/build/vendors~app.86e74a86d4afc6733e22.js:262932:20)
    at AppContainer (eval at ES6ProxyComponentFactory (http://localhost:3000/public/build/vendors~app.86e74a86d4afc6733e22.js:262445:10), <anonymous>:5:7)
    at HotExportedConnect(QueryRow) (http://localhost:3000/public/build/vendors~app.86e74a86d4afc6733e22.js:264803:9)
    at div
    at QueryRows (http://localhost:3000/public/build/explore.86e74a86d4afc6733e22.js:19380:5)
    at div
    at div
    at Explore (http://localhost:3000/public/build/explore.86e74a86d4afc6733e22.js:15795:5)
    at WithTheme(undefined)
    at Connect(WithTheme(undefined)) (http://localhost:3000/public/build/vendors~app.86e74a86d4afc6733e22.js:262932:20)
    at AppContainer (eval at ES6ProxyComponentFactory (http://localhost:3000/public/build/vendors~app.86e74a86d4afc6733e22.js:262445:10), <anonymous>:5:7)
    at HotExportedConnect(WithTheme(undefined)) (http://localhost:3000/public/build/vendors~app.86e74a86d4afc6733e22.js:264803:9)
    at div
    at ExplorePaneContainerUnconnected (eval at ES6ProxyComponentFactory (http://localhost:3000/public/build/vendors~app.86e74a86d4afc6733e22.js:262445:10), <anonymous>:5:7)
    at Connect(ExplorePaneContainerUnconnected) (http://localhost:3000/public/build/vendors~app.86e74a86d4afc6733e22.js:266974:75)
    at ErrorBoundary (http://localhost:3000/public/build/default~app.86e74a86d4afc6733e22.js:40077:5)
    at ErrorBoundaryAlert (http://localhost:3000/public/build/default~app.86e74a86d4afc6733e22.js:40128:5)
    at div
    at div
    at div
    at Scrollbars (http://localhost:3000/public/build/vendors~app.86e74a86d4afc6733e22.js:229434:9)
    at CustomScrollbar (http://localhost:3000/public/build/default~app.86e74a86d4afc6733e22.js:37435:28)
    at div
    at Wrapper (eval at ES6ProxyComponentFactory (http://localhost:3000/public/build/vendors~app.86e74a86d4afc6733e22.js:262445:10), <anonymous>:5:7)
    at Connect(Wrapper) (http://localhost:3000/public/build/vendors~app.86e74a86d4afc6733e22.js:266974:75)
    at LoadableComponent (http://localhost:3000/public/build/vendors~app.86e74a86d4afc6733e22.js:266051:7)
    at GrafanaRoute (http://localhost:3000/public/build/default~app.86e74a86d4afc6733e22.js:90892:5)
    at Route (eval at ES6ProxyComponentFactory (http://localhost:3000/public/build/vendors~app.86e74a86d4afc6733e22.js:262445:10), <anonymous>:5:7)
    at Switch (http://localhost:3000/public/build/vendors~app.86e74a86d4afc6733e22.js:269341:29)
    at div
    at Router (http://localhost:3000/public/build/vendors~app.86e74a86d4afc6733e22.js:268764:30)
    at div
    at ModalsProvider (http://localhost:3000/public/build/default~app.86e74a86d4afc6733e22.js:51400:5)
    at ThemeProvider (http://localhost:3000/public/build/default~app.86e74a86d4afc6733e22.js:96231:23)
    at ErrorBoundary (http://localhost:3000/public/build/default~app.86e74a86d4afc6733e22.js:40077:5)
    at ErrorBoundaryAlert (http://localhost:3000/public/build/default~app.86e74a86d4afc6733e22.js:40128:5)
    at Provider (http://localhost:3000/public/build/vendors~app.86e74a86d4afc6733e22.js:266687:20)
    at AppWrapper (eval at ES6ProxyComponentFactory (http://localhost:3000/public/build/vendors~app.86e74a86d4afc6733e22.js:262445:10), <anonymous>:5:7)

React will try to recreate this component tree from scratch using the error boundary you provided, AppContainer.
logCapturedError @ react-dom.development.js:20085
react-hot-loader.development.js:232 TypeError: Object(...) is not a function
    at Trigger.render (index.js:685)
    at finishClassComponent (react-dom.development.js:17485)
    at updateClassComponent (react-dom.development.js:17435)
    at beginWork (react-dom.development.js:19073)
    at HTMLUnknownElement.callCallback (react-dom.development.js:3945)
    at Object.invokeGuardedCallbackDev (react-dom.development.js:3994)
    at invokeGuardedCallback (react-dom.development.js:4056)
    at beginWork$1 (react-dom.development.js:23964)
    at performUnitOfWork (react-dom.development.js:22776)
    at workLoopSync (react-dom.development.js:22707)
error @ react-hot-loader.development.js:232
react-hot-loader.development.js:2145 Uncaught TypeError: Object(...) is not a function
    at Trigger.render (index.js:685)
    at finishClassComponent (react-dom.development.js:17485)
    at updateClassComponent (react-dom.development.js:17435)
    at beginWork (react-dom.development.js:19073)
    at HTMLUnknownElement.callCallback (react-dom.development.js:3945)
    at Object.invokeGuardedCallbackDev (react-dom.development.js:3994)
    at invokeGuardedCallback (react-dom.development.js:4056)
    at beginWork$1 (react-dom.development.js:23964)
    at performUnitOfWork (react-dom.development.js:22776)
    at workLoopSync (react-dom.development.js:22707)
react-dom.development.js:20085 The above error occurred in the <AppContainer> component:

    at AppContainer (eval at ES6ProxyComponentFactory (http://localhost:3000/public/build/vendors~app.86e74a86d4afc6733e22.js:262445:10), <anonymous>:5:7)
    at HotExportedConnect(QueryRow) (http://localhost:3000/public/build/vendors~app.86e74a86d4afc6733e22.js:264803:9)
    at div
    at QueryRows (http://localhost:3000/public/build/explore.86e74a86d4afc6733e22.js:19380:5)
    at div
    at div
    at Explore (http://localhost:3000/public/build/explore.86e74a86d4afc6733e22.js:15795:5)
    at WithTheme(undefined)
    at Connect(WithTheme(undefined)) (http://localhost:3000/public/build/vendors~app.86e74a86d4afc6733e22.js:262932:20)
    at AppContainer (eval at ES6ProxyComponentFactory (http://localhost:3000/public/build/vendors~app.86e74a86d4afc6733e22.js:262445:10), <anonymous>:5:7)
    at HotExportedConnect(WithTheme(undefined)) (http://localhost:3000/public/build/vendors~app.86e74a86d4afc6733e22.js:264803:9)
    at div
    at ExplorePaneContainerUnconnected (eval at ES6ProxyComponentFactory (http://localhost:3000/public/build/vendors~app.86e74a86d4afc6733e22.js:262445:10), <anonymous>:5:7)
    at Connect(ExplorePaneContainerUnconnected) (http://localhost:3000/public/build/vendors~app.86e74a86d4afc6733e22.js:266974:75)
    at ErrorBoundary (http://localhost:3000/public/build/default~app.86e74a86d4afc6733e22.js:40077:5)
    at ErrorBoundaryAlert (http://localhost:3000/public/build/default~app.86e74a86d4afc6733e22.js:40128:5)
    at div
    at div
    at div
    at Scrollbars (http://localhost:3000/public/build/vendors~app.86e74a86d4afc6733e22.js:229434:9)
    at CustomScrollbar (http://localhost:3000/public/build/default~app.86e74a86d4afc6733e22.js:37435:28)
    at div
    at Wrapper (eval at ES6ProxyComponentFactory (http://localhost:3000/public/build/vendors~app.86e74a86d4afc6733e22.js:262445:10), <anonymous>:5:7)
    at Connect(Wrapper) (http://localhost:3000/public/build/vendors~app.86e74a86d4afc6733e22.js:266974:75)
    at LoadableComponent (http://localhost:3000/public/build/vendors~app.86e74a86d4afc6733e22.js:266051:7)
    at GrafanaRoute (http://localhost:3000/public/build/default~app.86e74a86d4afc6733e22.js:90892:5)
    at Route (eval at ES6ProxyComponentFactory (http://localhost:3000/public/build/vendors~app.86e74a86d4afc6733e22.js:262445:10), <anonymous>:5:7)
    at Switch (http://localhost:3000/public/build/vendors~app.86e74a86d4afc6733e22.js:269341:29)
    at div
    at Router (http://localhost:3000/public/build/vendors~app.86e74a86d4afc6733e22.js:268764:30)
    at div
    at ModalsProvider (http://localhost:3000/public/build/default~app.86e74a86d4afc6733e22.js:51400:5)
    at ThemeProvider (http://localhost:3000/public/build/default~app.86e74a86d4afc6733e22.js:96231:23)
    at ErrorBoundary (http://localhost:3000/public/build/default~app.86e74a86d4afc6733e22.js:40077:5)
    at ErrorBoundaryAlert (http://localhost:3000/public/build/default~app.86e74a86d4afc6733e22.js:40128:5)
    at Provider (http://localhost:3000/public/build/vendors~app.86e74a86d4afc6733e22.js:266687:20)
    at AppWrapper (eval at ES6ProxyComponentFactory (http://localhost:3000/public/build/vendors~app.86e74a86d4afc6733e22.js:262445:10), <anonymous>:5:7)

React will try to recreate this component tree from scratch using the error boundary you provided, AppContainer.
logCapturedError @ react-dom.development.js:20085
react-hot-loader.development.js:232 TypeError: Object(...) is not a function
    at Trigger.render (index.js:685)
    at finishClassComponent (react-dom.development.js:17485)
    at updateClassComponent (react-dom.development.js:17435)
    at beginWork (react-dom.development.js:19073)
    at HTMLUnknownElement.callCallback (react-dom.development.js:3945)
    at Object.invokeGuardedCallbackDev (react-dom.development.js:3994)
    at invokeGuardedCallback (react-dom.development.js:4056)
    at beginWork$1 (react-dom.development.js:23964)
    at performUnitOfWork (react-dom.development.js:22776)
    at workLoopSync (react-dom.development.js:22707)
error @ react-hot-loader.development.js:232
react-hot-loader.development.js:2145 Uncaught TypeError: Object(...) is not a function
    at Trigger.render (index.js:685)
    at finishClassComponent (react-dom.development.js:17485)
    at updateClassComponent (react-dom.development.js:17435)
    at beginWork (react-dom.development.js:19073)
    at HTMLUnknownElement.callCallback (react-dom.development.js:3945)
    at Object.invokeGuardedCallbackDev (react-dom.development.js:3994)
    at invokeGuardedCallback (react-dom.development.js:4056)
    at beginWork$1 (react-dom.development.js:23964)
    at performUnitOfWork (react-dom.development.js:22776)
    at workLoopSync (react-dom.development.js:22707)
react-dom.development.js:20085 The above error occurred in the <AppContainer> component:

    at AppContainer (eval at ES6ProxyComponentFactory (http://localhost:3000/public/build/vendors~app.86e74a86d4afc6733e22.js:262445:10), <anonymous>:5:7)
    at HotExportedConnect(WithTheme(undefined)) (http://localhost:3000/public/build/vendors~app.86e74a86d4afc6733e22.js:264803:9)
    at div
    at ExplorePaneContainerUnconnected (eval at ES6ProxyComponentFactory (http://localhost:3000/public/build/vendors~app.86e74a86d4afc6733e22.js:262445:10), <anonymous>:5:7)
    at Connect(ExplorePaneContainerUnconnected) (http://localhost:3000/public/build/vendors~app.86e74a86d4afc6733e22.js:266974:75)
    at ErrorBoundary (http://localhost:3000/public/build/default~app.86e74a86d4afc6733e22.js:40077:5)
    at ErrorBoundaryAlert (http://localhost:3000/public/build/default~app.86e74a86d4afc6733e22.js:40128:5)
    at div
    at div
    at div
    at Scrollbars (http://localhost:3000/public/build/vendors~app.86e74a86d4afc6733e22.js:229434:9)
    at CustomScrollbar (http://localhost:3000/public/build/default~app.86e74a86d4afc6733e22.js:37435:28)
    at div
    at Wrapper (eval at ES6ProxyComponentFactory (http://localhost:3000/public/build/vendors~app.86e74a86d4afc6733e22.js:262445:10), <anonymous>:5:7)
    at Connect(Wrapper) (http://localhost:3000/public/build/vendors~app.86e74a86d4afc6733e22.js:266974:75)
    at LoadableComponent (http://localhost:3000/public/build/vendors~app.86e74a86d4afc6733e22.js:266051:7)
    at GrafanaRoute (http://localhost:3000/public/build/default~app.86e74a86d4afc6733e22.js:90892:5)
    at Route (eval at ES6ProxyComponentFactory (http://localhost:3000/public/build/vendors~app.86e74a86d4afc6733e22.js:262445:10), <anonymous>:5:7)
    at Switch (http://localhost:3000/public/build/vendors~app.86e74a86d4afc6733e22.js:269341:29)
    at div
    at Router (http://localhost:3000/public/build/vendors~app.86e74a86d4afc6733e22.js:268764:30)
    at div
    at ModalsProvider (http://localhost:3000/public/build/default~app.86e74a86d4afc6733e22.js:51400:5)
    at ThemeProvider (http://localhost:3000/public/build/default~app.86e74a86d4afc6733e22.js:96231:23)
    at ErrorBoundary (http://localhost:3000/public/build/default~app.86e74a86d4afc6733e22.js:40077:5)
    at ErrorBoundaryAlert (http://localhost:3000/public/build/default~app.86e74a86d4afc6733e22.js:40128:5)
    at Provider (http://localhost:3000/public/build/vendors~app.86e74a86d4afc6733e22.js:266687:20)
    at AppWrapper (eval at ES6ProxyComponentFactory (http://localhost:3000/public/build/vendors~app.86e74a86d4afc6733e22.js:262445:10), <anonymous>:5:7)

React will try to recreate this component tree from scratch using the error boundary you provided, ErrorBoundary.
logCapturedError @ react-dom.development.js:20085
time.ts:79 Uncaught (in promise) TypeError: Cannot read property 'range' of undefined
    at time.ts:79
    at index.js:8
    at dispatch (redux.js:638)
    at _callee$ (explorePane.ts:161)
    at u (testing-library.js:1)
    at Generator._invoke (testing-library.js:1)
    at Generator.forEach.e.<computed> [as next] (testing-library.js:1)
    at asyncGeneratorStep (default~app.86e74a86d4afc6733e22.js:120218)
    at _next (default~app.86e74a86d4afc6733e22.js:120220)

@leeoniya
Copy link
Contributor Author

leeoniya commented Apr 1, 2021

odd 🤔 , maybe a version mismatch between one local node_module and another global node_module?

@leeoniya
Copy link
Contributor Author

leeoniya commented Apr 1, 2021

internet says maybe to update stuff.

gaearon/react-hot-loader#1309

@dprokop
Copy link
Member

dprokop commented Apr 1, 2021

I can confirm @hugohaggmark finding. I am not sure this is hot-loader related. Did you have a chance to test the prod build @leeoniya against these changes?

@hugohaggmark hugohaggmark dismissed their stale review April 1, 2021 12:47

I'm removing the Request Changes because I'm going to be offline until Tuesday

@torkelo
Copy link
Member

torkelo commented Apr 2, 2021

The problem is the hoisting of all the emotion libraries that storybook depend on and storybook depends on emotion 10, while the app depends on 11. This hoisting takes precedence so the app ends up with emotion 10 sub libs (@emotion/sheet etc)

@leeoniya
Copy link
Contributor Author

leeoniya commented Apr 2, 2021

the storybook issue should be different than Explore crashing, though, right?

let's wait for @jackw to land #32638 for storybook, though 6.2 still looks to be on Emotion 10 :(

@torkelo
Copy link
Member

torkelo commented Apr 2, 2021

Think the problem could be fixed by adding the emotion libs to the nohoist list in package.json

@leeoniya
Copy link
Contributor Author

leeoniya commented Apr 7, 2021 •
edited

with all the recent upgrades an improvements (perhaps #32715), it looks like these changes are no longer necessary 🎉

immutable.js and eventemitter3 are the only remaining duplications left inside vendors/packages, but let's investigate that separately.

image

@leeoniya leeoniya closed this Apr 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hugohaggmark hugohaggmark hugohaggmark left review comments

@torkelo torkelo Awaiting requested review from torkelo

@ryantxu ryantxu Awaiting requested review from ryantxu

@dprokop dprokop Awaiting requested review from dprokop

Assignees
No one assigned
Labels
area/frontend
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@leeoniya @dprokop @torkelo @hugohaggmark @grafanabot