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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃敟 add base tsconfig #1829

Merged
merged 2 commits into from Jul 24, 2019

Conversation

@michenly
Copy link
Contributor

commented Jul 12, 2019

WHY are these changes introduced?

We now have base typescript configurations that project can pull in.
I am using polaris here as a test for library project.

Note that @shopify/typescript-configs actually come with the latest sewing-kit.
However I am too afraid to update sewing-kit myself so I pull in @shopify/typescript-configs separately instead.

WHAT is this pull request doing?

How to 馃帺

馃枼 Local development instructions
馃棐 General tophatting guidelines
馃搫 Changelog guidelines

As long as type-check passed we are all good!

Copy-paste this code in playground/Playground.tsx:
import * as React from 'react';
import {Page} from '@shopify/polaris';

interface State {}

export default class Playground extends React.Component<never, State> {
  render() {
    return (
      <Page title="Playground">
        {/* Add the code you want to test here */}
      </Page>
    );
  }
}

馃帺 checklist

@BPScott BPScott temporarily deployed to polaris-react-pr-1829 Jul 12, 2019 Inactive

@BPScott

This comment has been minimized.

Copy link
Member

commented Jul 12, 2019

Reminding myself to look at this when you're ready

@michenly michenly force-pushed the add-base-tsconfig branch from 7b7e526 to 3b2331b Jul 15, 2019

@BPScott BPScott temporarily deployed to polaris-react-pr-1829 Jul 15, 2019 Inactive

@michenly michenly force-pushed the add-base-tsconfig branch from 3b2331b to bc08900 Jul 17, 2019

@BPScott BPScott temporarily deployed to polaris-react-pr-1829 Jul 17, 2019 Inactive

@michenly michenly force-pushed the add-base-tsconfig branch from bc08900 to f5a6630 Jul 17, 2019

@BPScott BPScott temporarily deployed to polaris-react-pr-1829 Jul 17, 2019 Inactive

@michenly michenly force-pushed the add-base-tsconfig branch from f5a6630 to e923382 Jul 17, 2019

@BPScott BPScott temporarily deployed to polaris-react-pr-1829 Jul 17, 2019 Inactive

@michenly michenly force-pushed the add-base-tsconfig branch from e923382 to dc6b34c Jul 22, 2019

@BPScott BPScott temporarily deployed to polaris-react-pr-1829 Jul 22, 2019 Inactive

@michenly michenly requested review from BPScott and AndrewMusgrave Jul 23, 2019

@michenly michenly force-pushed the add-base-tsconfig branch from dc6b34c to 8501f0b Jul 23, 2019

@michenly michenly marked this pull request as ready for review Jul 23, 2019

@BPScott BPScott temporarily deployed to polaris-react-pr-1829 Jul 23, 2019 Inactive

@BPScott

This comment has been minimized.

Copy link
Member

commented Jul 23, 2019

However I am too afraid to update sewing-kit myself so I pull in @shopify/typescript-configs separately instead.

We use SK for our testing and linting scaffolding, we don't use it for builds so it's a pretty low impact change. I'll try and spin up a new PR to update to latest SK (to check theres no unrelated changes), then we can rebase this change atop that once it's merged. How does that sound?

@BPScott BPScott referenced this pull request Jul 23, 2019

@michenly michenly force-pushed the add-base-tsconfig branch from 8501f0b to 9e8dd7e Jul 23, 2019

@BPScott BPScott temporarily deployed to polaris-react-pr-1829 Jul 23, 2019 Inactive

@michenly

This comment has been minimized.

Copy link
Contributor Author

commented Jul 24, 2019

Rebased with the master!

@BPScott
Copy link
Member

left a comment

Fantastic stuff, thanks @michelleyschen! Build output is identical.

I learnt that you don't need the .json at the end of the extends declaration. I pushed a tiny change to make that be "extends": "@shopify/typescript-configs/library"

@BPScott BPScott merged commit a135283 into master Jul 24, 2019

10 checks passed

CLA Contributor License Agreement (CLA) status
Details
WIP ready for review
Details
changelog changelog entry included
Details
ci/circleci: a11y Your tests passed on CircleCI!
Details
ci/circleci: check Your tests passed on CircleCI!
Details
ci/circleci: percy Your tests passed on CircleCI!
Details
codecov/patch Coverage not affected when comparing faa039b...0dae23d
Details
percy/polaris-react Visual review automatically approved, no visual changes found.
Details
shrink-ray Webpack build report complete :)
Details
translation-platform Good to go
Details

@BPScott BPScott deleted the add-base-tsconfig branch Jul 24, 2019

@BPScott BPScott referenced this pull request Jul 24, 2019
@AndrewMusgrave

This comment has been minimized.

Copy link
Member

commented Jul 25, 2019

Late to the party but LGTM 馃帀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can鈥檛 perform that action at this time.