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

[web] Directly import createElement #8773

Merged

Conversation

EvanBacon
Copy link
Contributor

@EvanBacon EvanBacon commented Jun 11, 2020

@EvanBacon EvanBacon added the Platform: web Using Expo in the browser label Jun 11, 2020
@EvanBacon EvanBacon requested a review from ide June 11, 2020 02:02
@EvanBacon EvanBacon self-assigned this Jun 11, 2020
@github-actions
Copy link
Contributor

github-actions bot commented Jun 11, 2020

Fails
🚫

📋 Missing Changelog

🛠 Add missing entries to:

🛠 Suggested fixes:

📋 Missing changelog

Apply suggested changes:

diff --git a/packages/expo-gl/CHANGELOG.md b/packages/expo-gl/CHANGELOG.md
index be43473b..2ee984a9 100644
--- a/packages/expo-gl/CHANGELOG.md
+++ b/packages/expo-gl/CHANGELOG.md
@@ -10,6 +10,8 @@
 
 ### 🐛 Bug fixes
 
+- [web] Directly import createElement. ([#8773](https://github.com/expo/expo/pull/8773) by [@EvanBacon](https://github.com/EvanBacon))
+
 ## 8.3.1 — 2020-05-29
 
 *This version does not introduce any user-facing changes.*

Generated by 🚫 dangerJS against 1ab59e9

@github-actions
Copy link
Contributor

Native Component List for this branch is ready

@EvanBacon EvanBacon marked this pull request as ready for review June 12, 2020 20:36
Copy link
Contributor

@bbarthec bbarthec left a comment

Choose a reason for hiding this comment

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

LGTM, but what about this possibility to break snack?
Do we care here about it or you'll fix it later? 🤔
Is it possible to test it locally? 🤔

@@ -7,12 +7,13 @@
### 🎉 New features

- [av] Delete `prop-types` in favor of TypeScript. ([#8679](https://github.com/expo/expo/pull/8679) by [@EvanBacon](https://github.com/EvanBacon))
- [av] Directly import `createElement` from `react-native-web` for RNW v12 support. ([#8773](https://github.com/expo/expo/pull/8773) by [@EvanBacon](https://github.com/EvanBacon))
Copy link
Member

Choose a reason for hiding this comment

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

I think we don't need to prefix entries by the package name like [av] 🤔 That's kinda obvious they refer to that package. Also, the script that merges changelog into SDK changelog groups them by package like here: https://github.com/expo/expo/blob/master/CHANGELOG.md#-bug-fixes-1

Suggested change
- [av] Directly import `createElement` from `react-native-web` for RNW v12 support. ([#8773](https://github.com/expo/expo/pull/8773) by [@EvanBacon](https://github.com/EvanBacon))
- Directly import `createElement` from `react-native-web` for RNW v12 support. ([#8773](https://github.com/expo/expo/pull/8773) by [@EvanBacon](https://github.com/EvanBacon))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the changelog danger script added the prefix

@EvanBacon EvanBacon merged commit 3ee1058 into master Jun 16, 2020
@EvanBacon EvanBacon deleted the @evanbacon/react-native-web/direct-import-create-element branch June 16, 2020 01:34
Jamedjo pushed a commit to Jamedjo/expo that referenced this pull request Feb 4, 2021
* Directly import createElement to avoid name change issue in react-native-web@0.12

* Update CHANGELOG.md

* Update CHANGELOG.md
prakashbask pushed a commit to prakashbask/expo that referenced this pull request Mar 16, 2022
* Directly import createElement to avoid name change issue in react-native-web@0.12

* Update CHANGELOG.md

* Update CHANGELOG.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Platform: web Using Expo in the browser
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants