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: add support for Unistyles 3.0 #456

Merged
merged 11 commits into from
Feb 21, 2025
Merged

Conversation

jpudysz
Copy link
Contributor

@jpudysz jpudysz commented Feb 20, 2025

Expo Router:

  • - Stack
  • - Tabs
  • - Drawer + Tabs

React Navigation:

  • - Stack
  • - Tabs
  • - Drawer + Tabs

Other:

  • - None

@dannyhw @danstepanov PR is ready to be tested! I checked all UI templates mentioned above.

It would be good to merge it in a few days after I release beta.8. It should land on Tue or Wed next week.

@jpudysz jpudysz changed the title [WIP] feat: add support for Unistyles 3.0 feat: add support for Unistyles 3.0 Feb 20, 2025
@dannyhw
Copy link
Collaborator

dannyhw commented Feb 21, 2025

putting a checklist here for my testing

Expo Router:

  • - Stack
  • - Tabs
  • - Drawer + Tabs

React Navigation:

  • - Stack
  • - Tabs
  • - Drawer + Tabs

@dannyhw
Copy link
Collaborator

dannyhw commented Feb 21, 2025

@jpudysz I got an error from running ios, seems related to nitromodules on
bun run dev myTestProject --expo-router --stack --unistyles

@jpudysz
Copy link
Contributor Author

jpudysz commented Feb 21, 2025

Most likely you're on Xcode 16.2.
jpudysz/react-native-unistyles#507

@dannyhw
Copy link
Collaborator

dannyhw commented Feb 21, 2025

@jpudysz ah yeah I am, whats the fix in that case? Just downgrading?

@jpudysz
Copy link
Contributor Author

jpudysz commented Feb 21, 2025

If you add config plugin then it works on 16.2, but fails on 16.0.
I stick to 16.0 😅

@danstepanov
Copy link
Collaborator

@dannyhw I trust you to merge when you feel this is good to go

@dannyhw
Copy link
Collaborator

dannyhw commented Feb 21, 2025

Maybe this doesn't make sense but is there a way we could check the version of xcode and apply the plugin if they have 16.2?

if thats not viable then its not a big deal. I mean xcode breaking is a classic after all

@jpudysz
Copy link
Contributor Author

jpudysz commented Feb 21, 2025

I don't think, so. Sadly there is no fix for now and it's on Apple 😢

@dannyhw
Copy link
Collaborator

dannyhw commented Feb 21, 2025

thoughts on adding this is in the printoutput file

  if (stylingPackage?.name === 'unistyles' && os.type() === 'Darwin') {
    const xcodeVersion = await runSystemCommand({
      command: `xcodebuild -version | head -n 1 | awk '{print $2}'`,
      errorMessage: 'failed to check xcode version',
      stdio: 'pipe',
      toolbox,
      failOnError: false
    });

    const xcodeVersionString = String(xcodeVersion).trim();

    if (xcodeVersionString === '16.2') {
      warning(
        '\nUnistyles is currently not compatible with xcode 16.2 due to an xcode bug, downgrade to 16.1 or lower to use Unistyles \nhttps://github.com/jpudysz/react-native-unistyles/issues/507'
      );
    }
  }

I can push this change up if its acceptable to you

image

@jpudysz
Copy link
Contributor Author

jpudysz commented Feb 21, 2025

Wow so cool!!

@dannyhw
Copy link
Collaborator

dannyhw commented Feb 21, 2025

ok added it :), I have to go out but will test downgrading xcode etc later today

@dannyhw
Copy link
Collaborator

dannyhw commented Feb 21, 2025

can confirm that xcode 16.1 is working as expected for bun run dev myTestProject --expo-router --stack --unistyles --overwrite

@@ -114,12 +111,12 @@ export default function NotFoundScreen() {
fontSize: 14,
color: '#2e78b7',
},
});
}));
Copy link
Collaborator

Choose a reason for hiding this comment

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

An extra parentheses was added here which was breaking the tests. I will push a fix to remove. Just leaving this note to let you know what I changed.

Suggested change
}));
});

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah right, wrong template. It's for StyleSheet. Thanks for spotting it out

@dannyhw
Copy link
Collaborator

dannyhw commented Feb 21, 2025

after making the one fix for stylesheet + expo router stack everything seems to be good to go

@dannyhw dannyhw merged commit 23deeea into roninoss:main Feb 21, 2025
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.

3 participants