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

Feature: replace environmental variable with in binary expression #7954

Conversation

Shinyaigeek
Copy link
Contributor

↪️ Pull Request

Fixes: #7904

I implemented the feature to replace in binary expression ,whose right branch is process.env and left branch is string literal value, with boolean literal value.

💻 Examples

"SOME_VAR" in process.env

into

true // or false

🚨 Test instructions

prepare the test case to cover this.

✔️ PR Todo

  • Added/updated unit tests for this change
  • Filled out test instructions (In case there aren't any unit tests)
  • Included links to related issues/PRs

Comment on lines 52 to 62
if self.env.contains_key(&left.value) {
return Expr::Lit(Lit::Bool(Bool {
value: true,
span: DUMMY_SP,
}));
} else {
return Expr::Lit(Lit::Bool(Bool {
value: false,
span: DUMMY_SP,
}));
}
Copy link
Member

Choose a reason for hiding this comment

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

You can remove the if by doing

					return Expr::Lit(Lit::Bool(Bool {
                      value: self.env.contains_key(&left.value),
                      span: DUMMY_SP,
                    }));

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Certainly! Thanks! 👍 9b66c82

if let Expr::Ident(ref ident) = &*member.obj {
if !self.decls.contains(&id!(ident)) && &ident.sym == "process" {
if let MemberProp::Ident(ref prop) = member.prop {
if &prop.sym == "env" {
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should use the match_member_expr utility to match process.env as below?

packages/transformers/js/core/src/env_replacer.rs Outdated Show resolved Hide resolved
@devongovett devongovett force-pushed the feature/replace-env-in-binary-expression branch from 2f1c460 to d4ffae8 Compare April 21, 2022 13:57
Copy link
Member

@devongovett devongovett left a comment

Choose a reason for hiding this comment

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

Thanks! I did a little simplification to the code, hope you don't mind. 😄

@Shinyaigeek
Copy link
Contributor Author

I misunderstood that match_member_expr cannot be used in this usecase... Thanks!

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.

import from process left behind in browser-targeted output
3 participants