newrelic / nr1-workshop

Self-paced training workshop for the NR1 CLI/SDK
Apache License 2.0
86 stars 68 forks source link

React 16 dialect / functional components? #85

Closed JonathanMerklin closed 4 years ago

JonathanMerklin commented 4 years ago

Mostly bikeshedding, not an "issue" per se, but I noticed that despite the fact that you're leveraging "react": "^16.6.3", you've written a bunch of class-based components throughout this tutorial and using the "classic" component lifecycle, which is fine and perhaps an intentional decision for legacy clients that are more used to it (I know my current org is maintaining a mix of React 0.x/15.x/16.x).

But it may be worth considering that as the "canonical way to write React" gradually shifts towards hooks and the like, it may be worth updating the tutorial as well, lest it appear dated and ward off your potential customers.

As a simple example translation, I don't think I see any reason why the lab1 The resulting index.js should look like the following: can't be (modulo export default -> export to permit the arrow function and not a class)

// NOTE: I have not executed this, I'm just whipping it up to write this issue as an example
import React, { useState, useEffect, } from "react";
import { TableChart, Stack, StackItem, ChartGroup, LineChart, ScatterChart } from "nr1";

const accountId = "...";
export const Lab1Nerdlet = (props) => {
       const [ appData, setAppData, ] = useState({
          appId: null,
          appName: null,
        });
        useEffect(() => {
            console.debug("executes with similar timing to when the Nerdlet constructor would have");
  , []);
        const setApplication = (inAppId, inAppName) => {
          setAppData({appId: inAppId, appName: inAppName, });
        };
        const { appId, appName } = this.state;
        const nrql = `SELECT count(*) as 'transactions', apdex(duration) as 'apdex', percentile(duration, 99, 90, 70) FROM Transaction facet appName, appId limit 25`;
        const tCountNrql = `SELECT count(*) FROM Transaction WHERE appId = ${appId} TIMESERIES`;
        const apdexNrql = `SELECT apdex(duration) FROM Transaction WHERE appId = ${appId} TIMESERIES`
        //return the JSX we're rendering
        return (
            <ChartGroup>
                <Stack
                    verticalType={Stack.VERTICAL_TYPE.FILL}
                    directionType={Stack.DIRECTION_TYPE.VERTICAL}
                    gapType={Stack.GAP_TYPE.EXTRA_LOOSE}>
                    <StackItem>
                            <TableChart query={nrql} accountId={accountId} className="chart" onClickTable={(dataEl, row, chart) => {
                                //for learning purposes, we'll write to the console.
                                console.debug([dataEl, row, chart]) //eslint-disable-line
                                this.setApplication(row.appId, row.appName)
                            }}/>
                    </StackItem>
                    {appId && <StackItem>
                        <Stack
                            horizontalType={Stack.HORIZONTAL_TYPE.FILL}
                            directionType={Stack.DIRECTION_TYPE.HORIZONTAL}
                            gapType={Stack.GAP_TYPE.EXTRA_LOOSE}>
                            <StackItem>
                                <LineChart accountId={accountId} query={tCountNrql} className="chart"/>
                            </StackItem>
                            <StackItem>
                                <ScatterChart accountId={accountId} query={apdexNrql} className="chart"/>
                            </StackItem>
                        </Stack>
                    </StackItem>}
                </Stack>
            </ChartGroup>
        )
};

(as opposed to what it is as of this date i.e.)

import React from 'react';
import { TableChart, Stack, StackItem, ChartGroup, LineChart, ScatterChart } from 'nr1';

export default class Lab1Nerdlet extends React.Component {

    constructor(props) {
        super(props);
        this.accountId = <REPLACE_WITH_YOUR_ACCOUNT_ID>;
        this.state = {
            appId: null,
            appName: null
        };
        console.debug("Nerdlet constructor", this); //eslint-disable-line
    }

    setApplication(inAppId, inAppName) {
        this.setState({ appId: inAppId, appName: inAppName })
    }

    render(){
        const { appId, appName } = this.state;
        const nrql = `SELECT count(*) as 'transactions', apdex(duration) as 'apdex', percentile(duration, 99, 90, 70) FROM Transaction facet appName, appId limit 25`;
        const tCountNrql = `SELECT count(*) FROM Transaction WHERE appId = ${appId} TIMESERIES`;
        const apdexNrql = `SELECT apdex(duration) FROM Transaction WHERE appId = ${appId} TIMESERIES`
        //return the JSX we're rendering
        return (
            <ChartGroup>
                <Stack
                    verticalType={Stack.VERTICAL_TYPE.FILL}
                    directionType={Stack.DIRECTION_TYPE.VERTICAL}
                    gapType={Stack.GAP_TYPE.EXTRA_LOOSE}>
                    <StackItem>
                            <TableChart query={nrql} accountId={this.accountId} className="chart" onClickTable={(dataEl, row, chart) => {
                                //for learning purposes, we'll write to the console.
                                console.debug([dataEl, row, chart]) //eslint-disable-line
                                this.setApplication(row.appId, row.appName)
                            }}/>
                    </StackItem>
                    {appId && <StackItem>
                        <Stack
                            horizontalType={Stack.HORIZONTAL_TYPE.FILL}
                            directionType={Stack.DIRECTION_TYPE.HORIZONTAL}
                            gapType={Stack.GAP_TYPE.EXTRA_LOOSE}>
                            <StackItem>
                                <LineChart accountId={this.accountId} query={tCountNrql} className="chart"/>
                            </StackItem>
                            <StackItem>
                                <ScatterChart accountId={this.accountId} query={apdexNrql} className="chart"/>
                            </StackItem>
                        </Stack>
                    </StackItem>}
                </Stack>
            </ChartGroup>
        )
    }
}

Again, mostly bikeshedding, but I figured I'd raise the issue. If this is intentional and there is a technical reason for using the lifecycle methods, feel free to close! :)

jaesius commented 4 years ago

Hi Jonathan,

Thanks for your consideration and for taking the time to logs this issue. Currently, we don't support React hooks with building NR1 apps, but we have this update on our roadmap and once complete the workshop will get an update.

Thanks!