Skip to content

Use primer/react-scripts toolchain - #2046

Closed
siddharthkp wants to merge 30 commits into
mainfrom
siddharth/primer-scripts
Closed

Use primer/react-scripts toolchain#2046
siddharthkp wants to merge 30 commits into
mainfrom
siddharth/primer-scripts

Conversation

@siddharthkp

@siddharthkpsiddharthkp commented Apr 25, 2022

Copy link
Copy Markdown
Member

Part of the toolchain for component development

puppers eating food

Before we start asking other teams to start using the toolchain,
we should use it in internal repos to test the experience

Replaces local storybook setup with primer/react-scripts. Still allows for project specific customisation. See .storybook/main.js and .storybook/preview.js

The canary version is intentional, @primer/react-scripts is still a proof of concept and does not publish a stable version yet. The package name and script name might change. Now using stable version :)

Similar PR in memex: https://github.com/github/memex/pull/9638

@changeset-bot

changeset-botBot commented Apr 25, 2022

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 5d01542

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@siddharthkpsiddharthkp changed the title Use primer/react-scriptsUse primer/react-scripts toolchianApr 25, 2022
@siddharthkpsiddharthkp changed the title Use primer/react-scripts toolchianUse primer/react-scripts toolchainApr 25, 2022
@github-actions

github-actionsBot commented Apr 25, 2022

Copy link
Copy Markdown
Contributor

size-limit report 📦

PathSize
dist/browser.esm.js67.58 KB (-0.03% 🔽)
dist/browser.umd.js67.98 KB (+0.01% 🔺)

@siddharthkp
siddharthkp temporarily deployed to github-pages April 25, 2022 12:30 Inactive
@siddharthkpsiddharthkp self-assigned this Apr 25, 2022
@siddharthkpsiddharthkp added the skip changeset This change does not need a changelog label Apr 25, 2022
@siddharthkp
siddharthkp marked this pull request as ready for review April 25, 2022 12:33
@siddharthkp
siddharthkp requested a review from a teamApril 25, 2022 12:33
@siddharthkp
siddharthkp temporarily deployed to github-pages April 25, 2022 12:47 Inactive
@siddharthkp
siddharthkp temporarily deployed to needs-approval April 25, 2022 12:52 Inactive
</ThemeProvider>
)
}
],

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

We do this in every story, which creates nested ThemeProviders! Switching theme at the top level doesn't go all the way down :/

Needs some clean up after this PR is merged.

Comment threadsrc/utils/story-helpers.tsx
@siddharthkp
siddharthkp marked this pull request as draft April 25, 2022 13:02
@siddharthkp
siddharthkp temporarily deployed to github-pages April 25, 2022 13:09 Inactive
@siddharthkp
siddharthkp temporarily deployed to github-pages June 1, 2022 13:51 Inactive
@colebemis

Copy link
Copy Markdown
Contributor

@colebemis Do you know why CI is failing on this, looks like an unrelated change but it doens't fail on main 🤔

Could be because of a potential eslint version change?

Huh, I wonder if it's related to these changes: #2083

@github-actions

Copy link
Copy Markdown
Contributor

Hi! This pull request has been marked as stale because it has been open with no activity for 60 days. You can comment on the pull request or remove the stale label to keep it open. If you do nothing, this pull request will be closed in 7 days.

@github-actions
github-actionsBot deleted the siddharth/primer-scripts branch September 24, 2022 10:04
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

reactskip changesetThis change does not need a changelogStale

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@siddharthkp@colebemis