Skip to content

Scenes: Updating to v6 #1019

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

Merged
merged 22 commits into from
Apr 17, 2025
Merged

Scenes: Updating to v6 #1019

merged 22 commits into from
Apr 17, 2025

Conversation

leventebalogh
Copy link
Contributor

@leventebalogh leventebalogh commented Jan 28, 2025

Note

This is PR in its current form is mostly for demonstration purposes, I would leave the final touches and the merging to the @grafana/observability-logs team (of course happy to help in case there are any questions or issues).

What changed?

This PR updates to use scenes v6.0.0.

Notes for the reviewers

Please keep in mind that the PR is still depending on the canary version, which should be updated before merged. There are also a few temporary @ts-ignore statements that needs to be fixed before merging.

@leventebalogh leventebalogh added the scenes For issues that need to be addressed upstream in scenes framework label Jan 28, 2025
@leventebalogh leventebalogh self-assigned this Jan 28, 2025
@leventebalogh leventebalogh requested a review from a team as a code owner January 28, 2025 07:22
@gtk-grafana
Copy link
Contributor

This is currently breaking the entire app 😢

Seems to be getting thrown on this line on the third render:

const [isInitialized, setIsInitialized] = React.useState(false);

Error thrown in Grafana

Error: Element type is invalid: expected a string (for built-in components) or a class/function (for composite components) but got: undefined. You likely forgot to export your component from the file it's defined in, or you might have mixed up default and named imports.

Error thrown when debugging:

TypeError: Cannot read properties of null (reading 'useState')
    at Object.useState (react.development.js:1622:1)
    at LogExplorationView (LogExplorationPage.tsx:19:45)
    at describeNativeComponentFrame (react-dom.development.js:1092:1)
    at describeFunctionComponentFrame (react-dom.development.js:1187:1)
    at describeFiber (react-dom.development.js:1266:1)
    at getStackByFiberInDevAndProd (react-dom.development.js:1285:1)
    at getCurrentFiberStackInDev (react-dom.development.js:1522:1)
    at push../node_modules/react/cjs/react.development.js.ReactDebugCurrentFrame.getStackAddendum (react.development.js:130:1)
    at printWarning (react.development.js:193:1)
    at error (react.development.js:183:1)

@leventebalogh
Copy link
Contributor Author

Yeah, I know 😢

I am looking into it, will get back once I have found the root cause @gtk-grafana 👍 .

@leventebalogh
Copy link
Contributor Author

Hey @gtk-grafana 👋 I have managed to make the app work locally after doing some more updates, and the tests pass as well now. (The PR is not ready, as there are still a few temporary @ts-ignore lines in there, but apart from those I think it's fine).

Please let me know if you encounter any issues. 🙏

@gtk-grafana
Copy link
Contributor

@leventebalogh everything seems to work as expected, I removed the ts-ignores.
I do see a new error in the webpack output however:

WARNING in ../node_modules/react-router-dom-v5-compat/dist/index.js 1480:16-26
export 'useHistory' (imported as 'useHistory') was not found in 'react-router-dom' (possible exports: AbortedDeferredError, Await, BrowserRouter, Form, HashRouter, Link, MemoryRouter, NavLink, Navigate, NavigationType, Outlet, Route, Router, RouterProvider, Routes, ScrollRestoration, UNSAFE_DataRouterContext, UNSAFE_DataRouterStateContext, UNSAFE_ErrorResponseImpl, UNSAFE_FetchersContext, UNSAFE_LocationContext, UNSAFE_NavigationContext, UNSAFE_RouteContext, UNSAFE_ViewTransitionContext, UNSAFE_useRouteId, UNSAFE_useScrollRestoration, createBrowserRouter, createHashRouter, createMemoryRouter, createPath, createRoutesFromChildren, createRoutesFromElements, createSearchParams, defer, generatePath, isRouteErrorResponse, json, matchPath, matchRoutes, parsePath, redirect, redirectDocument, renderMatches, replace, resolvePath, unstable_HistoryRouter, unstable_usePrompt, useActionData, useAsyncError, useAsyncValue, useBeforeUnload, useBlocker, useFetcher, useFetchers, useFormAction, useHref, useInRouterContext, useLinkClickHandler, useLoaderData, useLocation, useMatch, useMatches, useNavigate, useNavigation, useNavigationType, useOutlet, useOutletContext, useParams, useResolvedPath, useRevalidator, useRouteError, useRouteLoaderData, useRoutes, useSearchParams, useSubmit, useViewTransitionState)
 @ ../node_modules/@grafana/ui/dist/esm/components/Link/Link.js 3:0-60 8:29-35
 @ ../node_modules/@grafana/ui/dist/esm/index.js 171:0-49 171:0-49
 @ ./Components/Table/ColumnSelection/LogsTableActiveFields.tsx 56:0-41 93:18-27
 @ ./Components/Table/ColumnSelection/LogsTableMultiSelect.tsx 4:0-95 50:52-73
 @ ./Components/Table/ColumnSelection/ColumnSelectionDrawerWrap.tsx 57:0-93 192:42-62
 @ ./Components/Table/Table.tsx 93:0-121 145:26-42 287:41-66
 @ ./Components/Table/TableWrap.tsx 5:0-47 90:41-46
 @ ./Components/Table/TableProvider.tsx 2:0-55 18:41-50
 @ ./Components/ServiceScene/LogsTableScene.tsx 16:0-55 88:54-67
 @ ./Components/ServiceScene/LogsListScene.tsx 55:0-50 187:30-44
 @ ./Components/ServiceScene/LogOptionsScene.tsx 34:0-48 66:48-61
 @ ./services/datasource.ts 97:0-119 291:26-54 291:60-90
 @ ./module.tsx 38:45-74

Deleting node_modules and re-installing doesn't seem to fix.

@gtk-grafana gtk-grafana added this to the 1.0.10 milestone Feb 27, 2025
@gtk-grafana
Copy link
Contributor

@leventebalogh do you know more about the above, it's worrisome that we're not using v5 but it's still throwing warnings when building

@L2D2Grafana
Copy link
Collaborator

L2D2Grafana commented Apr 3, 2025

@leventebalogh everything seems to work as expected, I removed the ts-ignores. I do see a new error in the webpack output however:

WARNING in ../node_modules/react-router-dom-v5-compat/dist/index.js 1480:16-26
export 'useHistory' (imported as 'useHistory') was not found in 'react-router-dom' (possible exports: AbortedDeferredError, Await, BrowserRouter, Form, HashRouter, Link, MemoryRouter, NavLink, Navigate, NavigationType, Outlet, Route, Router, RouterProvider, Routes, ScrollRestoration, UNSAFE_DataRouterContext, UNSAFE_DataRouterStateContext, UNSAFE_ErrorResponseImpl, UNSAFE_FetchersContext, UNSAFE_LocationContext, UNSAFE_NavigationContext, UNSAFE_RouteContext, UNSAFE_ViewTransitionContext, UNSAFE_useRouteId, UNSAFE_useScrollRestoration, createBrowserRouter, createHashRouter, createMemoryRouter, createPath, createRoutesFromChildren, createRoutesFromElements, createSearchParams, defer, generatePath, isRouteErrorResponse, json, matchPath, matchRoutes, parsePath, redirect, redirectDocument, renderMatches, replace, resolvePath, unstable_HistoryRouter, unstable_usePrompt, useActionData, useAsyncError, useAsyncValue, useBeforeUnload, useBlocker, useFetcher, useFetchers, useFormAction, useHref, useInRouterContext, useLinkClickHandler, useLoaderData, useLocation, useMatch, useMatches, useNavigate, useNavigation, useNavigationType, useOutlet, useOutletContext, useParams, useResolvedPath, useRevalidator, useRouteError, useRouteLoaderData, useRoutes, useSearchParams, useSubmit, useViewTransitionState)
 @ ../node_modules/@grafana/ui/dist/esm/components/Link/Link.js 3:0-60 8:29-35
 @ ../node_modules/@grafana/ui/dist/esm/index.js 171:0-49 171:0-49
 @ ./Components/Table/ColumnSelection/LogsTableActiveFields.tsx 56:0-41 93:18-27
 @ ./Components/Table/ColumnSelection/LogsTableMultiSelect.tsx 4:0-95 50:52-73
 @ ./Components/Table/ColumnSelection/ColumnSelectionDrawerWrap.tsx 57:0-93 192:42-62
 @ ./Components/Table/Table.tsx 93:0-121 145:26-42 287:41-66
 @ ./Components/Table/TableWrap.tsx 5:0-47 90:41-46
 @ ./Components/Table/TableProvider.tsx 2:0-55 18:41-50
 @ ./Components/ServiceScene/LogsTableScene.tsx 16:0-55 88:54-67
 @ ./Components/ServiceScene/LogsListScene.tsx 55:0-50 187:30-44
 @ ./Components/ServiceScene/LogOptionsScene.tsx 34:0-48 66:48-61
 @ ./services/datasource.ts 97:0-119 291:26-54 291:60-90
 @ ./module.tsx 38:45-74

Deleting node_modules and re-installing doesn't seem to fix.

I think that grafana/ui is looking for
react-router-dom-v5-compat in the yarn.lock, I believe if grafana/ui is update to the latest this error would go away.

Copy link
Contributor

@tskarhed tskarhed left a comment

Choose a reason for hiding this comment

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

Please click around and verify that everything works :)

@gtk-grafana
Copy link
Contributor

Everything appears to be working in the application, but I'm seeing this in the console:

instrumentation.js:52 You rendered descendant <Routes> (or called `useRoutes()`) at "/a/grafana-lokiexplore-app/explore/service/tempo-ingester/logs" (under <Route path=":labelName/:labelValue/logs">) but the parent route path has no trailing "*". This means if you navigate deeper, the parent won't match anymore and therefore the child routes will never render.

Please change the parent <Route path=":labelName/:labelValue/logs"> to <Route path=":labelName/:labelValue/logs/*">.

@gtk-grafana
Copy link
Contributor

@leventebalogh @L2D2Grafana is going to take this over and hopefully get this over the finish line

@gtk-grafana gtk-grafana modified the milestones: 1.0.10, 1.0.12 Apr 8, 2025
@leventebalogh
Copy link
Contributor Author

@gtk-grafana Sorry for not being responsive on this lately, I got snowed under with other plugins-platform tasks that needed to be finished before G12. Sure and thanks a lot!

@gtk-grafana gtk-grafana removed this from the 1.0.12 milestone Apr 8, 2025
@L2D2Grafana
Copy link
Collaborator

L2D2Grafana commented Apr 8, 2025

Interesting I'm now seeing the .setShowMenuAlways(true) not working for LogsPanelScene.tsx however it does seem to work for the Histogram. Not sure what's up yet, but I think that's breaking the tests.
Screenshot 2025-04-08 at 2 51 14 PM

Update
scenes made showMenuAlways conditional on the collapsible property. We could set all out panels to be collapsible or I opened a PR to make all the props independently conditional.

@L2D2Grafana
Copy link
Collaborator

Alright updated to [email protected] let's go 🚀!

@@ -33,12 +33,11 @@ export default defineConfig<PluginOptions>({


/* Collect trace when retrying the failed test. See https://playwright.dev/docs/trace-viewer */
trace: 'on-first-retry',

trace: 'on',
Copy link
Contributor

Choose a reason for hiding this comment

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

on-first-retry makes the initial execution quite a bit faster, and less resource intensive

@@ -13,6 +13,6 @@ export class LogsSceneQueryRunner extends SceneQueryRunner {

// @todo can we make runWithTimeRange protected? (https://github.com/grafana/scenes/pull/866)
// Hack to call private method
this['runWithTimeRange'](timeRange);
this['runWithTimeRangeAndScopes'](timeRange);
Copy link
Contributor

Choose a reason for hiding this comment

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

oofdah, I should have cleaned this up by now, created #1201 to track my shame

Copy link
Contributor

@gtk-grafana gtk-grafana 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, works well locally.

@L2D2Grafana L2D2Grafana merged commit a13d9e2 into main Apr 17, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scenes For issues that need to be addressed upstream in scenes framework
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants