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(replace): typeofReplacements option #1084

Merged
merged 13 commits into from
Feb 11, 2022
Merged

feat(replace): typeofReplacements option #1084

merged 13 commits into from
Feb 11, 2022

Conversation

guybedford
Copy link
Contributor

@guybedford guybedford commented Jan 8, 2022

Rollup Plugin Name: plugin-replace

This PR contains:

  • bugfix
  • feature
  • refactor
  • documentation
  • other

Are tests included?

  • yes (bugfixes and features will not be merged without tests)
  • no

Breaking Changes?

  • yes (breaking changes will not be merged unless absolutely necessary)
  • no

Description

Currently process.env.NODE_ENV replacements only replace that exact string. But as came up in reactjs/react-transition-group#777, it can be useful for browser code to guard these replacements like:

if (typeof process !== 'undefined' && process.env.NODE_ENV === 'production') {...}

by default this plugin will not handle these other guards properly.

This PR adds a new typeofReplacements: true option (disabled by default, although it would be nice to enable by default in future), which will replace the typeof process with "object" in the case of identifier member expression replacement strings.

The user only needs to define process.env.NODE_ENV as the replacement key, and it will automatically expand the parent object typeof replacements when using this option.

See the README for a further example.

@guybedford guybedford requested a review from shellscape as a code owner January 8, 2022 00:48
@guybedford
Copy link
Contributor Author

guybedford commented Jan 8, 2022

It's very tricky to get these snapshots to line up when working on Windows. Everything is passing, but would be appreciated if someone with a unix machine could update the snapshots here. Alternatively when I next have access to WSL I can take a look.

Update: Finally got the snapshots updated over WSL on my other machine.

@shellscape shellscape changed the title feat(plugin-replace): typeofReplacements option feat(replace): typeofReplacements option Jan 9, 2022
@shellscape
Copy link
Collaborator

The concept is sound. Not so sure about the option name. To anyone not familiar with the use case, I don't think the option name conveys what the option does.

@guybedford
Copy link
Contributor Author

Thanks for the review @shellscape I've renamed the feature to objectGuards if that seems like a more descriptive name?

@shellscape
Copy link
Collaborator

Sounds good to me

@guybedford guybedford merged commit 2d769ac into master Feb 11, 2022
@guybedford guybedford deleted the replace-typeof branch February 11, 2022 22:37

> Snapshot 1

`if (typeof process !== 'undefined' && "production" === 'production') {␊
Copy link

@privatenumber privatenumber Apr 29, 2022

Choose a reason for hiding this comment

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

@guybedford

Shouldn't typeof process be transformed to 'object'? :

if ('object' !== 'undefined' && "production" === 'production') {

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