Skip to content

Safe import for immutable#367

Open
elsaperelli wants to merge 1 commit into
masterfrom
safe-immutable-import
Open

Safe import for immutable#367
elsaperelli wants to merge 1 commit into
masterfrom
safe-immutable-import

Conversation

@elsaperelli
Copy link
Copy Markdown
Contributor

Pull requests into cql-execution require the following.
Submitter and reviewer should ✔ when done.
For items that are not-applicable, mark "N/A" and ✔.

Summary

This PR updates the immutable imports that were causing issues down the line when cql-execution is used as a dependency in a browser-bundling application. This was not clear with Node through CommonJS because it worked, but the Immutable ESM build that bundled browser environments use exposes Set (and others) as named exports. Therefore, this PR fixes errors that Set was undefined.

Submitter:

  • This pull request describes why these changes were made
  • Code diff has been done and been reviewed (it does not contain: additional white space, not applicable code changes, debug statements, etc.)
  • Tests are included and test edge cases
  • Tests have been run locally and pass
  • Code coverage has not gone down and all code touched or added is covered.
  • Code passes lint and prettier (hint: use npm run test:plus to run tests, lint, and prettier)
  • All dependent libraries are appropriately updated or have a corresponding PR related to this change
  • cql4browsers.js built with npm run build:browserify if source changed.

Reviewer:

Name:

  • Code is maintainable and reusable, reuses existing code and infrastructure where appropriate, and accomplishes the task’s purpose
  • The tests appropriately test the new code, including edge cases
  • You have tried to break the code

@codecov-commenter
Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 88.23529% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.69%. Comparing base (997329d) to head (2be77e9).

Files with missing lines Patch % Lines
src/util/immutableUtil.ts 86.66% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master     #367   +/-   ##
=======================================
  Coverage   87.69%   87.69%           
=======================================
  Files          52       52           
  Lines        4624     4624           
  Branches     1307     1307           
=======================================
  Hits         4055     4055           
  Misses        354      354           
  Partials      215      215           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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.

2 participants