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

Future - Drop Emotion 10 types in lib/theming #18598

Merged
merged 26 commits into from
Jul 19, 2022

Conversation

ndelangen
Copy link
Member

No description provided.

@nx-cloud
Copy link

nx-cloud bot commented Jun 29, 2022

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 5571712. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch


✅ Successfully ran 1 target

Sent with 💌 from NxCloud.

@ndelangen ndelangen self-assigned this Jun 29, 2022
@ndelangen ndelangen closed this Jun 29, 2022
@ndelangen ndelangen reopened this Jun 29, 2022
# Conflicts:
#	lib/theming/src/emotion10types.ts
@ndelangen ndelangen changed the base branch from future/base to future/tsup-libs-1 June 29, 2022 07:47
@ndelangen ndelangen added the maintenance User-facing maintenance tasks label Jun 29, 2022
@ndelangen ndelangen changed the base branch from future/tsup-libs-1 to future/pbm/stage0 June 29, 2022 08:02
@ndelangen ndelangen requested a review from Andarist June 29, 2022 08:07
@ndelangen
Copy link
Member Author

ndelangen commented Jun 29, 2022

@Andarist OK, this is getting into shape now, I think.

I think it's prepped for a proper integration now. But the outputted types aren;t correct. We're getting the empty Theme interface in our components now, not the Theme interface we define.

Screen Shot 2022-06-29 at 10 10 39

Screen Shot 2022-06-29 at 10 10 58

We get this one:
Screen Shot 2022-06-29 at 10 13 26

Not this one 😢
Screen Shot 2022-06-29 at 10 13 46

We have some time to get this sorted, but I think it's a requirement for 7.0.

@ndelangen ndelangen changed the title Drop Emotion 10 types Future - Drop Emotion 10 types in lib/theming Jul 1, 2022
@Andarist
Copy link
Contributor

Andarist commented Jul 4, 2022

I believe that I got it working - both for prod bundles and for the dev environment. Please retest if it matches your expectations.

import { background, typography, color } from './base';
import { Theme, Color, ThemeVars } from './types';
import { StorybookTheme, Color, ThemeVars } from './types';
Copy link
Contributor

Choose a reason for hiding this comment

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

note that within the theming package you have to reference StorybookTheme and not Theme because Theme doesn't exist yet - it's only added after bundling and into the reexporting file

@@ -0,0 +1,9 @@
// this file is only actually used in development
Copy link
Contributor

Choose a reason for hiding this comment

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

Believe it or not but this has to be in a separate file and I can't reuse src/typings.d.ts - I've already stumbled upon a problem like this in the past, TS rules are weird when it comes to those d.ts files and I don't fully understand them. When put in the other file I couldn't use import at the top-level and without it, the tsup didn't recognize other Emotion exports (such as keyframes, css etc). This way I only touch what I actually want and leave the rest as-is

@ndelangen
Copy link
Member Author

I'll take a look tomorrow!

Base automatically changed from future/pbm/stage0 to future/base July 6, 2022 10:48
@ndelangen
Copy link
Member Author

  • Merged in future/base
  • Fixed the build
  • Cleanup up all usage of <{}> that served no purpose
  • Cleanup up use of FunctionComponent and replaced it with FC which is shorter
  • Extracted a few components wrapped directly with styled so their types are working properly

@ndelangen
Copy link
Member Author

@shilman can we take a look at the doc blocks UI changes being detected?

Som flacky styling (there's a underline underneath the "raw" text.

# Conflicts:
#	lib/blocks/src/blocks/DocsContainer.tsx
#	lib/blocks/src/blocks/SourceContainer.tsx
#	lib/blocks/src/blocks/Subtitle.tsx
#	lib/blocks/src/blocks/Title.tsx
#	lib/blocks/src/components/ColorPalette.tsx
#	lib/blocks/src/components/IconGallery.tsx
#	lib/blocks/src/components/Source.tsx
#	lib/blocks/src/components/Story.tsx
#	lib/components/src/placeholder/placeholder.tsx
#	lib/ui/src/components/preview/toolbar.tsx
#	lib/ui/src/components/preview/utils/types.tsx
#	lib/ui/src/components/sidebar/Sidebar.tsx
#	lib/ui/src/components/sidebar/TreeNode.tsx
@ndelangen ndelangen requested a review from shilman July 19, 2022 07:59
@ndelangen ndelangen merged commit e9655df into future/base Jul 19, 2022
@ndelangen ndelangen deleted the drop-emotion-10-types branch July 19, 2022 12:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance User-facing maintenance tasks theming
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants