Skip to content
This repository has been archived by the owner on Jun 13, 2023. It is now read-only.

fix(patcher.js): rollup does not build dynamic imports #20

Merged
merged 4 commits into from
Dec 9, 2018

Conversation

ranrib
Copy link
Member

@ranrib ranrib commented Dec 7, 2018

rollup does not compile dynamic imports (such as tryRequire) in the build.
It means that events/pg was never instrumented.

@ranrib ranrib requested a review from galbash December 7, 2018 10:59
Copy link
Member

@galbash galbash left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 small comment

src/events/pg.js Outdated
const sqlParser = require('node-sqlparser');
const pg = tryRequire('pg');
const Pool = tryRequire('pg-pool');
const sqlParser = tryRequire('node-sqlparser');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be a regular require

/* eslint-enable import/no-unresolved, import/no-extraneous-dependencies */
const sqlParser = require('node-sqlparser');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove the eslint line above

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@ranrib ranrib merged commit 84cd161 into master Dec 9, 2018
@ranrib ranrib deleted the fix-dynamic-imports branch December 9, 2018 14:24
@ranrib
Copy link
Member Author

ranrib commented Dec 9, 2018

🎉 This PR is included in version 1.4.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants