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(babel): minify typeof window #27530

Merged
merged 10 commits into from Mar 13, 2024
Merged

Conversation

EvanBacon
Copy link
Contributor

Why

It's common to use typeof window === 'undefined' to conditionally enable/disable code for server/client environments.

babel-preset-expo automatically transforms typeof window === 'undefined to true when bundling for server environments, and false when bundling for websites. The check is left as-is when bundling for native client environments to support apps that polyfill window.

This transform is run in both development and production, but only removes conditional requires in production.

You can configure babel-preset-expo to skip the transform by passing { preserveTypeofWindow: false }.

How

Add babel-plugin-transform-define and conditionally enable when bundling for websites and server code. Skip entirely for native client bundles as these are ambiguous. We also expose the ability to force the flag on/off for native client in case users want a more strict runtime protection against typeof window.

Additionally, I merged process.env.EXPO_OS and BASE_URL in this transform to reduce passes. In a follow up PR, I'll merge process.env.NODE_ENV and __DEV__ too.

Test Plan

  • Added a bunch of babel tests for this functionality.

Checklist

EvanBacon and others added 3 commits March 8, 2024 15:04
Co-authored-by: Expo Bot <34669131+expo-bot@users.noreply.github.com>
@expo-bot expo-bot added bot: passed checks ExpoBot has nothing to complain about and removed bot: suggestions ExpoBot has some suggestions labels Mar 8, 2024
Copy link
Member

@amandeepmittal amandeepmittal left a comment

Choose a reason for hiding this comment

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

Docs changes lgtm 👍

Left a few minor suggestions

docs/pages/guides/tree-shaking.mdx Outdated Show resolved Hide resolved
docs/pages/guides/tree-shaking.mdx Outdated Show resolved Hide resolved
docs/pages/guides/tree-shaking.mdx Outdated Show resolved Hide resolved
docs/pages/guides/tree-shaking.mdx Outdated Show resolved Hide resolved
@EvanBacon EvanBacon merged commit bc51c52 into main Mar 13, 2024
12 of 18 checks passed
@EvanBacon EvanBacon deleted the @evanbacon/babel/minify-typeof-window branch March 13, 2024 17:22
EvanBacon added a commit that referenced this pull request Mar 13, 2024
# Why

- The upstream module has a lot of concessions for legacy features that
don't appear to be used anymore, like importing from a module named
`require("Platform")` (Haste).
- We also want to combine the dev inlining with our other inlines via
#27530
- I've also added correct handling for web, where we'll now remove the
`native` fallback option.
- Finally, I added support for `Platform["OS"]`.

<!--
Please describe the motivation for this PR, and link to relevant GitHub
issues, forums posts, or feature requests.
-->

# Test Plan

- Added a bunch of tests for the expected functionality.

# Checklist

<!--
Please check the appropriate items below if they apply to your diff.
This is required for changes to Expo modules.
-->

- [ ] Documentation is up to date to reflect these changes (eg:
https://docs.expo.dev and README.md).
- [ ] Conforms with the [Documentation Writing Style
Guide](https://github.com/expo/expo/blob/main/guides/Expo%20Documentation%20Writing%20Style%20Guide.md)
- [ ] This diff will work correctly for `npx expo prebuild` & EAS Build
(eg: updated a module plugin).

---------

Co-authored-by: Expo Bot <34669131+expo-bot@users.noreply.github.com>
@brentvatne brentvatne added the published Changes from the PR have been published to npm label Apr 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot: fingerprint compatible bot: passed checks ExpoBot has nothing to complain about published Changes from the PR have been published to npm
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants