fix(build): fix define plugin spread operations

[pajter]

Description

I noticed a bug in the define plugin where the RegExp would not match when using spread operations. For instance, this would not get replaced:

const myObj = {
   ...import.meta.env.VITE_ENV === 'dev' ? { a: 1 } : {}
}

By adding another negative look-behind inside, it will accept ... but still reject ..

Additional context

n/a

---

What is the purpose of this pull request?

• Bug fix
• New Feature
• Documentation update
• Other

Before submitting the PR, please make sure you do the following

• Read the Contributing Guidelines.
• Read the Pull Request Guidelines and follow the Commit Convention.
• Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
• Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
• Ideally, include relevant tests that fail without this PR but pass with it.

[Shinigami92]

Why would 'process.env.SOMEOBJ': { spread: true } every be a possible value?
I think process.env.SOMETHING will ever be a string, something other than a string should not be allowed or be parse with JSON.parse

[pajter]

True. I hadn't really thought about that. It's a silly example.

I'll update it to be make more sense, such as: { ...process.env.NODE_ENV === 'dev' ? { dev: true } : {} }

[Shinigami92]

Lets say process.env.NODE_ENV is defined as 'dev'
Even then ...process.env.NODE_ENV === 'dev' would be false cause e.g. ...'dev' would spread the string into something like a char array 👀
So why and when would you need spread for a process env variable?

[pajter]

It's not spreading the env variable, but rather using it in a ternary operator that spreads another object. I was using it in a config object where I needed to add conditional config based on an env variable.

const config = {
  ...process.env.NODE_ENV === 'dev' ? { watch: true } : { minify: true }
}

I know that code could be written a lot of different ways, and I've since refactored it, but I thought I'd send in a PR since it took me some debugging and I figured someone else might run into the same issue at some point.

The same issue would appear when spreading something from env into an array, which might be more common than my use case. Something like const sequence = [ ...process.env.SOME_NUMBERS ].

[Shinigami92]

Wait... is

const config = {
  ...process.env.NODE_ENV === 'dev' ? { watch: true } : { minify: true }
}

even allowed JS?
Shouldn't it be at least e.g.

const config = {
  ...(process.env.NODE_ENV === 'dev' ? { watch: true } : { minify: true })
}

👀

---

And also const sequence = [ ...process.env.SOME_NUMBERS ] should already be allowed, isn't it?
I would think it will result in e.g. ['0', '1', '2'] where process.env.SOME_NUMBERS equals to '012'

So are you sure this PR is really fixing anything? And if so, what?
And then please update the tests that should fail without the changes.
And tests should not contain invalid usages of process.env

[pajter]

Wait... is

const config = {
  ...process.env.NODE_ENV === 'dev' ? { watch: true } : { minify: true }
}

even allowed JS?
Shouldn't it be at least e.g.

const config = {
  ...(process.env.NODE_ENV === 'dev' ? { watch: true } : { minify: true })
}

I got confused by that at first as well. But it's allowed.

And also const sequence = [ ...process.env.SOME_NUMBERS ] should already be allowed, isn't it?
I would think it will result in e.g. ['0', '1', '2'] where process.env.SOME_NUMBERS equals to '012'

It will still fail the RegExp because of the preceding .... I updated the tests to include that as well.

So are you sure this PR is really fixing anything? And if so, what?
And then please update the tests that should fail without the changes.
And tests should not contain invalid usages of process.env

I have cleaned the tests and verified that this fix does work. Both tests I added will fail without the addition to the RegExp:

After the RegExp fix: