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

feat: refactor iotsitewise source structure #67

Merged
merged 2 commits into from
Feb 25, 2022
Merged

Conversation

diehbria
Copy link
Contributor

@diehbria diehbria commented Feb 25, 2022

Overview

  • make widgetId optional, automatically assign a uuid if not present
  • move time-series and asset work to the source-iotsitewise package
  • refactor API to work as such:
import { initialize } from '@iot-app-kit/source-iotsitewise';
const { query } = initialize(...);

which removes the concept of a global iot app kit instance with credentials passed in.

  • Remove sitewise specific types throughout the iot-app-kit components.
  • introduced new generic types to represent TreeProvider and ProviderWithViewport
  • Simplify portions of the asset module - move away from wrapping everything as classes. Prefer to just pass in data. Classes should be utilized to maintain state.
  • add iot-table with support for table columns, utilizing exposed { toId } from '@iot-app-kit/source-iotsitewise'
  • alter build system to make rollup not yell about typescript
  • remove unecessary type aliases. We should not be type aliasing for every utilization of generics. We should just use the generics directly. Type aliasing every combination just adds indirection.

Remaining issues

  • failing snapshots on grid and status timeline - seem to have been failing for previous PRs as well.
  • failing test on expand row for iot-resource-explorer
  • open handles on the resource-explorer unit tests
  • need to re-write tests on composeProviders

Legal

This project is available under the Apache 2.0 License.

@diehbria diehbria marked this pull request as draft February 25, 2022 03:44
@diehbria diehbria force-pushed the refactor-iot-app-kit branch from 457e5e8 to 912ee9d Compare February 25, 2022 04:56
@@ -0,0 +1,72 @@
// import {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

need to rewrite this.

@diehbria diehbria force-pushed the refactor-iot-app-kit branch 3 times, most recently from 85f8fad to d93dd19 Compare February 25, 2022 17:25
import { ErrorDetails } from '../common/types';

export type TimeSeriesData = {
Copy link
Contributor

Choose a reason for hiding this comment

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

My feedback is that you are trying to re-use this type for both tables and charts. If you require additional data for a table you should make that part of the interface:

export type TimeSeriesTableData = {
  dataStreams: {id: string, stream: DataStream}[];
  viewport: MinimalViewPortConfig;
};

export type TimeSeriesTableData = {
  dataStreams: DataStream[];
  viewport: MinimalViewPortConfig;
};

maybe it makes sense to always have the IDs?

- bring back iot-table
- expose toId from source-iotsitewise
- move timeseries module to source-iotsitewise
- move asset module to source-iotsitewise
- remove sitewise explicit typining from components except resource
  explorer
- remove sitewise-resource-explorer
- refactor API so you initialize queries, instead of app kit instance
@diehbria diehbria marked this pull request as ready for review February 25, 2022 21:11
@diehbria diehbria merged commit ec1ba70 into main Feb 25, 2022
@diehbria diehbria deleted the refactor-iot-app-kit branch February 25, 2022 21:12
TheEvilDev pushed a commit that referenced this pull request Nov 2, 2022
- bring back iot-table
- expose toId from source-iotsitewise
- move timeseries module to source-iotsitewise
- move asset module to source-iotsitewise
- remove sitewise explicit typining from components except resource
  explorer
- remove sitewise-resource-explorer
- refactor API so you initialize queries, instead of app kit instance

Co-authored-by: Norbert Nader <Norbert.Nader@gmail.com>
This was referenced Nov 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants