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: create react-components and source-iotsitewise pkgs #57

Merged
merged 1 commit into from
Feb 16, 2022
Merged

Conversation

diehbria
Copy link
Contributor

@diehbria diehbria commented Feb 15, 2022

Overview

  • move core build files away from stencil config into package.json to
    prepare moving away from stencil to build core
  • fix various typing issues in iot-app-kit
  • remove unused code around creating a 'queue' of requests in the asset module. Not actually used - will add when/if we actually implement.
  • remove unnecessary AssetQuery type, which collides with other types.
  • fix bug where string resolution passed into error callback
  • remove global.d.ts overrides on jest-extended in favor of just importing jest-extended, as the custom matchers aren't actually used or implemented.
  • Upgrade to synchrocharts v1.1.1
  • Upgrade related table to v17 react
  • fix many 'open handles' being left in jest tests

Legal

This project is available under the Apache 2.0 License.

@diehbria diehbria marked this pull request as draft February 15, 2022 15:51
* move core build files away from stencil config into package.json to
  prepare moving away from stencil to build core
* fix various typing issues in iot-app-kit
* remove unused code
* remove unecessary `AssetQuery` type, which collides with other types
* make test runner fail if uncommited changes present
@diehbria diehbria marked this pull request as ready for review February 16, 2022 18:55
Copy link
Contributor

@boweihan boweihan left a comment

Choose a reason for hiding this comment

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

The approach looks good, few small suggestions.

],
plugins: [typescript({ tsconfig: './tsconfig.json' })],
},
{
Copy link
Contributor

Choose a reason for hiding this comment

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

yay rollup

@@ -3,7 +3,7 @@ import { sitewiseSdk } from './iotsitewise/time-series-data/sitewise-sdk';
import { SiteWiseAssetDataSource } from './data-module/types';
import { createSiteWiseAssetDataSource } from './iotsitewise/time-series-data/asset-data-source';
import { SiteWiseAssetModule } from './asset-modules';
import { IoTAppKitInitInputs, IoTAppKitComponentSession } from './interface.d';
import { IoTAppKitInitInputs, IoTAppKitComponentSession } from './index';
Copy link
Contributor

Choose a reason for hiding this comment

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

this could just be '.'

@@ -1015,4 +1021,6 @@ it('requests buffered data', async () => {
propertyId: query.assets[0].properties[0].propertyId,
})
);

unsubscribe();
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice cleanup!

"description": "React specific wrapper for IoT Application Kit",
"author": {
"name": "Amazon Web Services",
"url": "https://aws.amazon.com/"
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 standard? Should we be linking to AWS or can we link to SiteWise?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

grabbed this from other repos

@@ -0,0 +1,106 @@
import React, { createElement } from 'react';
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we leave a comment to the source at the top of file so it's clear we didn't write it? I assume we pulled this from somewhere like https://github.com/ionic-team/stencil-ds-output-targets/blob/main/packages/react-output-target/react-component-lib/createComponent.tsx.

@@ -0,0 +1,142 @@
import React from 'react';
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment wrt. documentation for this file

@@ -0,0 +1,114 @@
import { camelToDashCase } from './case';
Copy link
Contributor

Choose a reason for hiding this comment

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

Documentation of source

@diehbria diehbria merged commit 7b0f3cf into main Feb 16, 2022
@diehbria diehbria deleted the pkgs branch February 16, 2022 19:29
sheilaXu pushed a commit that referenced this pull request Sep 23, 2022
…lign with UX (#57)

Co-authored-by: Mitchell Lee <mitchlee@amazon.com>
TheEvilDev pushed a commit that referenced this pull request Nov 2, 2022
* move core build files away from stencil config into package.json to
  prepare moving away from stencil to build core
* fix various typing issues in iot-app-kit
* remove unused code
* remove unecessary `AssetQuery` type, which collides with other types
* make test runner fail if uncommited changes present
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.

None yet

3 participants