fix(rust): don't mark pure-annotated local calls as execution-order-sensitive#8909
fix(rust): don't mark pure-annotated local calls as execution-order-sensitive#8909
Conversation
… execution-order-sensitive Pure-annotated calls to local functions (e.g. `/*#__PURE__*/ foo()`) should not trigger `ExecutionOrderSensitive`, since the annotation is a developer contract that the call is side-effect-free. Only `Unknown` and `GlobalVarAccess` flags should trigger wrapping. This also removes the now-unused `SideEffectDetail::PureAnnotation` flag.
| //#endregion | ||
| __esmMin((() => { | ||
| init_dep(); | ||
| nodeAssert.strictEqual(globalThis.value, 1); |
There was a problem hiding this comment.
The diff is back to the state before #5498
There was a problem hiding this comment.
Since foo mutate the globalValue, it should be wrapped to ensure the correct execution order
There was a problem hiding this comment.
Thank you for reviewing. I indeed found a case where the mixed issue-4920 and issue-5303 can be broken after this change. Sorry for wasting your time.
There was a problem hiding this comment.
Thank you for reviewing. I indeed found a case where the mixed issue-4920 and issue-5303 can be broken after this change. Sorry for wasting your time.
Don't sweat it! It’s definitely not a waste of time—it was a tricky edge case. I'm just happy to help.
There was a problem hiding this comment.
Do you think it is worth adding a test?
✅ Deploy Preview for rolldown-rs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Merging this PR will not alter performance
Comparing Footnotes
|
…eck (#8933) ## Summary - Adds an integration test combining scenarios from #4920 and #5303 to demonstrate that `PureAnnotation` must be included in the `ExecutionOrderSensitive` check. - A `/*#__PURE__*/` call to a local function that reads a global variable set by another module must still trigger wrapping, otherwise execution order is broken. Ref: #8909 (comment) ## Test plan - [x] New test `pure_annotation_local_fn_reads_global` passes with `just dt` - [x] Snapshot confirms `dep.js` is wrapped with `__esmMin` in both default and `onDemandWrapping` variants 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Pure-annotated calls to local functions (e.g.
/*#__PURE__*/ foo()) should not triggerExecutionOrderSensitive, since the annotation is a developer contract that the call is side-effect-free. OnlyUnknownandGlobalVarAccessflags should trigger wrapping.This also removes the now-unused
SideEffectDetail::PureAnnotationflag.