-
Notifications
You must be signed in to change notification settings - Fork 14
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
Adds postcss-import plugin + fixes import for react phone styles #456
Conversation
New dependencies detected. Learn more about Socket for GitHub ↗︎
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
Let's make this high prio and patch release asap |
@@ -60,7 +64,7 @@ interface AuthProps { | |||
facebookEnabled: boolean; | |||
googleEnabled: boolean; | |||
}; | |||
configOrder: string[]; | |||
configOrder: AuthSection[]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's make these changes in another PR - this breaks example build since this type is not exported and will not be consistent with our docs at the moment
6ede7e1
to
eaf3e20
Compare
import url from "@rollup/plugin-url"; | ||
import alias from "@rollup/plugin-alias"; | ||
import copy from "rollup-plugin-copy"; | ||
import typescript from '@rollup/plugin-typescript'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would expect these to be double quotes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
from all the package READMES it shows single quotes actually. It seems like we were incorrect before
Summary & Motivation
CSS Import Rules Error
After importing
import '@turnkey/sdk-react/styles';
in thelayout.tsx
file and runningnpm run dev
, an error occurred. When bundling CSS with Rollup usingrollup-plugin-postcss
,@import
rules are not being correctly placed at the top of the final CSS bundle. This violates CSS specifications, which require@import
statements to precede all other rules (except@charset
and@layer
).Solution
Add
postcss-import
plugin to the PostCSS configuration to the Rollup setup.The
postcss-import
plugin processes@import rules
and ensures they appear at the top of the CSS file. By including this in the PostCSS plugins array, all@import
rules are handled correctly before other CSS rules are processed, preventing build errors.Additional Build Error with postcss-import
When using
postcss-import
, I encountered a build error related to importing CSS from external packages:postcss-import: Failed to find 'react-international-phone/style.css'
This occurs because
postcss-import
cannot resolve the CSS import from thenode_modules
directory when it's imported within a CSS file.Solution
Instead of importing the styles in
PhoneInput.css
:Move the import to the React component file (
PhoneInput.tsx
) directly:This approach allows the bundler to correctly resolve the CSS import from
node_modules
, fixing the build error.How I Tested These Changes
Did you add a changeset?
If updating one of our packages, you'll likely need to add a changeset to your PR. To do so, run
pnpm changeset
.pnpm changeset
will generate a file where you should write a human friendly message about the changes. Note how this (example) includes the package name (should be auto added by the command) along with the type of semver change (major.minor.patch) (which you should set).These changes will be used at release time to determine what packages to publish and how to bump their version. For more context see this comment.