feat: auto-require Postman sandbox globals during import#7878
Open
gopu-bruno wants to merge 3 commits intousebruno:mainfrom
Open
feat: auto-require Postman sandbox globals during import#7878gopu-bruno wants to merge 3 commits intousebruno:mainfrom
gopu-bruno wants to merge 3 commits intousebruno:mainfrom
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds a translator AST pass that detects Postman sandbox globals (CryptoJS, _, moment, cheerio, tv4) used as member-expression objects or call-expression callees, and injects corresponding Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
gopu-bruno
requested review from
bijin-bruno,
helloanoop,
lohit-bruno,
naman-bruno and
sid-bruno
as code owners
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
packages/bruno-converters/tests/postman/postman-translations/transpiler-tests/postman-library-globals.test.js (1)
16-31: Add a cheerio coverage case to close mapping/order gap.This suite validates four globals but misses
cheerio, which is part of the new auto-require feature. Add it to prevent silent mapping regressions.Proposed test update
it('injects multiple requires in alphabetical order', () => { const out = translateCode(` const h = CryptoJS.MD5('x').toString(); const c = _.chunk([1,2,3], 2); + const $ = cheerio.load('<div/>'); const t = moment().unix(); const v = tv4.validate({}, {}); `); + const idxCheerio = out.indexOf(`require("cheerio")`); const idxCrypto = out.indexOf(`require("crypto-js")`); const idxLodash = out.indexOf(`require("lodash")`); const idxMoment = out.indexOf(`require("moment")`); const idxTv4 = out.indexOf(`require("tv4")`); + expect(idxCheerio).toBeGreaterThan(-1); expect(idxCrypto).toBeGreaterThan(-1); + expect(idxCheerio).toBeLessThan(idxCrypto); expect(idxCrypto).toBeLessThan(idxLodash); expect(idxLodash).toBeLessThan(idxMoment); expect(idxMoment).toBeLessThan(idxTv4); });As per coding guidelines "Add tests for any new functionality or meaningful changes. If code is added, removed, or significantly modified, corresponding tests should be updated or created."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-converters/tests/postman/postman-translations/transpiler-tests/postman-library-globals.test.js` around lines 16 - 31, The test "injects multiple requires in alphabetical order" is missing coverage for cheerio; update the test that calls translateCode to include a cheerio usage (e.g., referencing cheerio.load or similar) and add an index check variable (like idxCheerio) using out.indexOf(`require("cheerio")`), then assert idxLodash < idxCheerio < idxMoment (or the correct alphabetical order among idxCrypto, idxLodash, idxCheerio, idxMoment, idxTv4) so the expectations verify cheerio is injected and sits in the proper alphabetical position; update the variable names and comparisons in the existing test (which uses translateCode and idxCrypto/idxLodash/idxMoment/idxTv4) to include idxCheerio accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/bruno-converters/src/utils/postman-to-bruno-translator.js`:
- Around line 699-703: The current isLibraryUsage check rejects computed
MemberExpression access (parent.computed), so patterns like CryptoJS['MD5'] or
_['chunk'] are ignored; update the logic in the isLibraryUsage assignment to
also accept computed member accesses when the property is a literal string
(e.g., parent.property.type === 'Literal' or 'StringLiteral' depending on the
parser), e.g., treat MemberExpression as library usage if parent.object ===
path.value and (not computed OR computed with a string/constant property). Keep
the existing CallExpression check unchanged (parent.type === 'CallExpression' &&
parent.callee === path.value).
- Line 722: The current transformation uses
ast.get().value.program.body.unshift(...declarations) which prepends
declarations before any existing directive prologue (e.g. 'use strict') and thus
loses strict mode; change the insertion to detect the directive prologue and
insert declarations after it — for example, inspect
ast.get().value.program.directives (or count leading ExpressionStatement string
literals in ast.get().value.program.body), compute the insert index as the
number of directives, and use Array.prototype.splice to insert declarations at
that index instead of unshift so directives remain at the top.
- Around line 713-720: The current generation of declarations sorts
usedLibraries by identifier instead of the actual package name, causing
non-deterministic require order; update the sort to compare
POSTMAN_LIBRARY_GLOBALS entries (e.g., [...usedLibraries].sort((a,b) =>
POSTMAN_LIBRARY_GLOBALS[a].localeCompare(POSTMAN_LIBRARY_GLOBALS[b]))) so the
variable declarations array (`declarations`) is built in deterministic
package-name order; adjust the sort call where `declarations` is created
(references: declarations, usedLibraries, POSTMAN_LIBRARY_GLOBALS).
---
Nitpick comments:
In
`@packages/bruno-converters/tests/postman/postman-translations/transpiler-tests/postman-library-globals.test.js`:
- Around line 16-31: The test "injects multiple requires in alphabetical order"
is missing coverage for cheerio; update the test that calls translateCode to
include a cheerio usage (e.g., referencing cheerio.load or similar) and add an
index check variable (like idxCheerio) using out.indexOf(`require("cheerio")`),
then assert idxLodash < idxCheerio < idxMoment (or the correct alphabetical
order among idxCrypto, idxLodash, idxCheerio, idxMoment, idxTv4) so the
expectations verify cheerio is injected and sits in the proper alphabetical
position; update the variable names and comparisons in the existing test (which
uses translateCode and idxCrypto/idxLodash/idxMoment/idxTv4) to include
idxCheerio accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6cef1592-7568-42c4-86e0-cc45b14942c0
📒 Files selected for processing (2)
packages/bruno-converters/src/utils/postman-to-bruno-translator.jspackages/bruno-converters/tests/postman/postman-translations/transpiler-tests/postman-library-globals.test.js
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes import failures from Postman to Bruno caused by missing sandbox globals (CryptoJS, _, moment, cheerio, tv4).
Tracked in : JIRA
Change
Adds a final AST-based pass in the Postman→Bruno translator to automatically inject the necessary require() statements. Imported collections run without manual fixes, improving migration reliability.
Contribution Checklist:
Note: Keeping the PR small and focused helps make it easier to review and merge. If you have multiple changes you want to make, please consider submitting them as separate pull requests.
Publishing to New Package Managers
Please see here for more information.
Summary by CodeRabbit
New Features
Tests