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.

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

Update rollup bundling config #33

Merged
merged 3 commits into from
Dec 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions .babelrc

This file was deleted.

1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,6 @@ cypress/report.json
cypress/screenshots/actual
cypress/videos/
dist/
compiled/
yarn-error.log
.idea/
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

All notable changes to this project will be documented in this file.

## v0.0.41

- Update rollup bundling config https://github.com/grafana/grafana-aws-sdk-react/pull/33

## v0.0.40

- Add support for optional labelWidth prop in Config components
Expand Down
7 changes: 0 additions & 7 deletions index.js

This file was deleted.

48 changes: 19 additions & 29 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,65 +1,55 @@
{
"name": "@grafana/aws-sdk",
"version": "0.0.40",
"version": "0.0.41",
"description": "Common AWS features for grafana",
"main": "dist/index.js",
"publishConfig": {
"main": "dist/index.js",
"module": "dist/esm/index.js",
"types": "dist/index.d.ts",
"access": "public"
},
"scripts": {
"dev": "watch 'npm run bundle' ./src",
"dev": "yarn bundle --watch",
"build": "yarn clean && yarn typecheck && yarn bundle",
"bundle": "rollup -c rollup.config.ts",
"clean": "rimraf ./dist ./compiled",
"typecheck": "tsc --noEmit",
"lint": "eslint --cache --ignore-path ./.gitignore --ext .js,.jsx,.ts,.tsx ./src",
"typecheck": "tsc -p ./tsconfig.build.json",
"test": "jest --notify --watch",
"test:coverage": "jest --coverage",
"test-ci": "grafana-toolkit plugin:test"
},
"files": [
"index.js",
"dist"
],
"repository": "github:grafana/grafana-aws-sdk-react",
"author": "Grafana Labs <[email protected]> (https://grafanap.com)",
Copy link
Member

Choose a reason for hiding this comment

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

😴

"author": "Grafana Labs <[email protected]> (https://grafana.com)",
"license": "Apache-2.0",
"dependencies": {},
"devDependencies": {
"@babel/preset-env": "^7.13.12",
"@babel/preset-react": "^7.13.13",
"@babel/preset-typescript": "^7.13.0",
"@grafana/data": "9.2.3",
"@grafana/runtime": "9.2.3",
"@grafana/toolkit": "9.2.3",
"@grafana/ui": "9.2.3",
"@rollup/plugin-commonjs": "11.0.2",
"@rollup/plugin-json": "4.0.3",
"@rollup/plugin-node-resolve": "7.1.1",
"@rollup/plugin-node-resolve": "^15.0.1",
"@testing-library/jest-dom": "5.16.5",
"@testing-library/react": "12.1.5",
"@testing-library/react-hooks": "8.0.1",
"@types/braintree__sanitize-url": "4.0.0",
"@types/jest": "27.4.1",
"@types/jquery": "3.3.38",
"@types/lodash": "4.14.123",
"@types/node": "10.14.1",
"@types/papaparse": "5.2.0",
"@types/lodash": "4.14.191",
"@types/node": "16.18.6",
"@types/react": "17.0.42",
"@types/react-dom": "17.0.14",
"@types/react-test-renderer": "^17.0.1",
"@types/rollup-plugin-visualizer": "2.6.0",
"@types/sinon": "^7.5.2",
"babel-jest": "^26.6.3",
"esbuild": "^0.16.2",
"jest": "27.5.1",
"node-notifier": "^10.0.1",
"pretty-format": "25.1.0",
"react": "17.0.2",
"react-dom": "17.0.2",
"react-select-event": "^5.3.0",
"react-test-renderer": "^17.0.2",
"rimraf": "^3.0.2",
"rollup": "2.0.6",
"rollup-plugin-sourcemaps": "0.5.0",
"rollup-plugin-terser": "5.3.0",
"rollup-plugin-typescript2": "^0.29.0",
"rollup-plugin-visualizer": "3.3.1",
"sinon": "8.1.1"
"rollup": "2.79.1",
"rollup-plugin-dts": "^5.0.0",
"rollup-plugin-esbuild": "^5.0.0",
"rollup-plugin-node-externals": "^5.0.2"
}
}
67 changes: 34 additions & 33 deletions rollup.config.ts
Original file line number Diff line number Diff line change
@@ -1,43 +1,44 @@
import resolve from '@rollup/plugin-node-resolve';
import commonjs from '@rollup/plugin-commonjs';
import { terser } from 'rollup-plugin-terser';
import path from 'path';
import dts from 'rollup-plugin-dts';
import esbuild from 'rollup-plugin-esbuild';
import { externals } from 'rollup-plugin-node-externals';

import typescript from 'rollup-plugin-typescript2';
const packagePath = './package.json';
const pkg = require(packagePath);

const pkg = require('./package.json');

const libraryName = pkg.name;

const buildCjsPackage = ({ env }) => {
return {
export default [
{
input: 'src/index.ts',
plugins: [
externals({
deps: true,
include: ['react', '@grafana/data', '@grafana/ui', 'lodash'],
packagePath,
}),
resolve(),
esbuild(),
],
output: [
{
file: `dist/index.${env}.js`,
name: libraryName,
format: 'cjs',
sourcemap: true,
chunkFileNames: `[name].${env}.js`,
strict: false,
exports: 'named',
globals: {
react: 'React',
'prop-types': 'PropTypes',
},
dir: path.dirname(pkg.publishConfig.main),
},
{
format: 'esm',
sourcemap: true,
dir: path.dirname(pkg.publishConfig.module),
preserveModules: true,
},
],
external: ['react', '@grafana/data', '@grafana/ui', 'lodash'],
plugins: [
typescript({
rollupCommonJSResolveHack: false,
clean: true,
}),
commonjs({
include: /node_modules/,
}),
resolve(),
env === 'production' && terser(),
],
};
};
export default [buildCjsPackage({ env: 'development' }), buildCjsPackage({ env: 'production' })];
},
{
input: './compiled/index.d.ts',
plugins: [dts()],
output: {
file: pkg.publishConfig.types,
format: 'es',
},
},
];
6 changes: 4 additions & 2 deletions src/ConnectionConfig.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,10 @@ import { awsAuthProviderOptions } from './providers';

const toOption = (value: string) => ({ value, label: value });

export interface ConnectionConfigProps<J = AwsAuthDataSourceJsonData, S = AwsAuthDataSourceSecureJsonData>
extends DataSourcePluginOptionsEditorProps<J, S> {
export interface ConnectionConfigProps<
J extends AwsAuthDataSourceJsonData = AwsAuthDataSourceJsonData,
S = AwsAuthDataSourceSecureJsonData
> extends DataSourcePluginOptionsEditorProps<J, S> {
standardRegions?: string[];
loadRegions?: () => Promise<string[]>;
defaultEndpoint?: string;
Expand Down
6 changes: 3 additions & 3 deletions src/index.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
export { ConnectionConfig, ConnectionConfigProps } from './ConnectionConfig';
export { ConnectionConfig, type ConnectionConfigProps } from './ConnectionConfig';
Copy link
Contributor

Choose a reason for hiding this comment

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

What does adding type here do?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In Typescript, types are erased because js doesn't actually have the same runtime concept of types like ts. But when code is being transpiled, this export statement is actually trying to find something exported from the ConnectionConfig module, but since it's erased, it doesn't actually find anything. Adding type here indicates that it's just importing a type and there's nothing actually exported.

export { SIGV4ConnectionConfig } from './SIGV4ConnectionConfig';
export { ConfigSelect, InlineInput } from './sql/ConfigEditor';
export { ResourceSelector, ResourceSelectorProps } from './sql/ResourceSelector';
export { SQLQuery } from './sql/types';
export { ResourceSelector, type ResourceSelectorProps } from './sql/ResourceSelector';
export { type SQLQuery } from './sql/types';
export { QueryCodeEditor, FormatSelect, FillValueSelect, FillValueOptions } from './sql/QueryEditor';
export * from './sql/utils';
export * from './types';
Expand Down
4 changes: 2 additions & 2 deletions src/providers.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { SelectableValue } from '@grafana/data';
import { AwsAuthType } from './types';

export const awsAuthProviderOptions = [
export const awsAuthProviderOptions: Array<SelectableValue<AwsAuthType>> = [
{
label: 'Workspace IAM Role',
value: AwsAuthType.EC2IAMRole,
Expand All @@ -18,4 +18,4 @@ export const awsAuthProviderOptions = [
label: 'Credentials file',
value: AwsAuthType.Credentials,
},
] as Array<SelectableValue<AwsAuthType>>;
];
5 changes: 2 additions & 3 deletions src/sql/ConfigEditor/ConfigSelect.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React from 'react';
import React, { DependencyList } from 'react';
Copy link
Member

Choose a reason for hiding this comment

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

👀 I don't think I've heard of this one!

Copy link
Contributor

Choose a reason for hiding this comment

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

👆🏻

import { DataSourcePluginOptionsEditorProps, SelectableValue } from '@grafana/data';
import { InputActionMeta } from '@grafana/ui';
import { AwsAuthDataSourceJsonData, AwsAuthDataSourceSecureJsonData } from '../../types';
Expand Down Expand Up @@ -47,7 +47,7 @@ export function ConfigSelect(props: ConfigSelectProps) {
className: 'width-30',
};
// Any change in the AWS connection details will affect selectors
const dependencies: string[] = [
const dependencies: DependencyList = [
props.options.jsonData.assumeRoleArn,
props.options.jsonData.authType,
props.options.jsonData.defaultRegion,
Expand All @@ -71,7 +71,6 @@ export function ConfigSelect(props: ConfigSelectProps) {
allowCustomValue={props.allowCustomValue}
autoFocus={props.autoFocus}
backspaceRemovesValue={props.backspaceRemovesValue}
className={props.className}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed passing the className prop down as the current behaviour is already overriding the className (see commonProps on line 43).

Copy link
Contributor

Choose a reason for hiding this comment

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

Should we still be passing 'width-30' as the className?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's still being passed by commonProps.

invalid={props.invalid}
isClearable={props.isClearable}
isMulti={props.isMulti}
Expand Down
4 changes: 2 additions & 2 deletions src/sql/ConfigEditor/index.ts
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
export { ConfigSelect, ConfigSelectProps } from './ConfigSelect';
export { InlineInput, InlineInputProps } from './InlineInput';
export { ConfigSelect, type ConfigSelectProps } from './ConfigSelect';
export { InlineInput, type InlineInputProps } from './InlineInput';
6 changes: 6 additions & 0 deletions src/sql/ResourceSelector.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ describe('ResourceSelector', () => {
render(<ResourceSelector {...props} default="foo" value={defaultKey} fetch={fetch} onChange={onChange} />);
expect(screen.queryByText('default (foo)')).toBeInTheDocument();

if (!props.label) {
throw new Error('label is required');
}
const selectEl = screen.getByLabelText(props.label);
expect(selectEl).toBeInTheDocument();

Expand All @@ -37,6 +40,9 @@ describe('ResourceSelector', () => {
const resources = ['foo', 'bar'];
render(<ResourceSelector {...props} fetch={undefined} onChange={onChange} resources={resources} />);

if (!props.label) {
throw new Error('label is required');
}
const selectEl = screen.getByLabelText(props.label);
expect(selectEl).toBeInTheDocument();

Expand Down
6 changes: 3 additions & 3 deletions src/sql/ResourceSelector.tsx
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import { SelectableValue } from '@grafana/data';
import { InlineField, Select, SelectCommonProps } from '@grafana/ui';
import { isEqual } from 'lodash';
import React, { useEffect, useMemo, useState } from 'react';
import React, { useEffect, useMemo, useState, DependencyList } from 'react';

import { defaultKey } from './types';

export interface ResourceSelectorProps extends SelectCommonProps<string> {
value: string | null;
dependencies?: Array<string | null | undefined>;
dependencies?: DependencyList;
tooltip?: string;
label?: string;
'data-testid'?: string;
Expand Down Expand Up @@ -82,7 +82,7 @@ export function ResourceSelector(props: ResourceSelectorProps) {
await props.saveOptions();
}
try {
const resources = await props.fetch();
const resources = (await props.fetch?.()) || [];
setResources(resources);
} finally {
setFetched(true);
Expand Down
4 changes: 4 additions & 0 deletions tsconfig.build.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"exclude": ["**/*.test.ts", "**/*.test.tsx", "dist", "node_modules"],
"extends": "./tsconfig.json"
}
11 changes: 6 additions & 5 deletions tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
{
"extends": "@grafana/tsconfig/base.json",
"include": ["src", "index.js"],
"extends": "@grafana/tsconfig",
"include": ["src/**/*.ts"],
"compilerOptions": {
"rootDir": "./src",
"baseUrl": "./src",
"rootDir": "./src",
"typeRoots": ["./node_modules/@types"],
"declaration": true,
"outDir": "dist"
"declarationDir": "./compiled",
"emitDeclarationOnly": true,
"isolatedModules": true
}
}
Loading