-
-
Notifications
You must be signed in to change notification settings - Fork 6.2k
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: allow any JS identifier in define, not ASCII-only #5972
Conversation
trailing dots in replacements are not supported in dev, just prod and only used internally for import.meta.env. (in dev it's handled by importAnalysisPlugin)
Rebased and updated to include the exclusion of (trailing) assignments shipped in 2.8: #5515 |
Greatly simplified the RegExp and fixed the bug where equality operators following an env var would break the replacement. (e.g. Tests pass now, and I think this is ready to be merged? |
bc6e44b
to
a12f327
Compare
LGTM, let's wait for other approvals. And IMO we should merge this one as part of the 3.0 beta, just to play safe. |
Co-authored-by: Bjorn Lu <bjornlu.dev@gmail.com>
Description
The
define
plugin uses RegExp\b
boundaries to make sure that it only replaces standalone identifiers (only_PROD_
, not_PROD_USE_DEVTOOLS_
). But\b
only works with ASCII characters and also not with$
, so it doesn't work reliably and worse, there can be false positives leading to broken code:This PR updates the RegExp to work with any valid JS identifier, so
$
and Unicode letters can be included. I also added a couple more tests, both for the newly added$
and Unicode functionality as well as for previously untested functionality:_OTHER_CONST_CONTAINING_PRODUCTION_
)someApiResponse._PRODUCTION_
)fix #5956
Additional context
Because the usage of
\b
for replacements was mentioned in the docs and therefore specced, this technically is a new feature and not a bugfix.What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123
).