Skip to content

Commit

Permalink
Suppress Circular Dependencies Warnings related to D3 (#1501)
Browse files Browse the repository at this point in the history
# Pull Request

## 🤨 Rationale

The d3 library chooses to intentionally keep circular dependencies:
d3/d3-selection#168

Rollup reports circular dependencies as warnings but to limit verbosity
only show the first few so all we see are warnings related to d3.
Example:

```
> rollup --bundleConfigAsCjs --config


dist/esm/all-components.js → dist/all-components-bundle.js...
(!) Circular dependencies
../../node_modules/d3-selection/src/selection/index.js -> ../../node_modules/d3-selection/src/selection/select.js -> ../../node_modules/d3-selection/src/selection/index.js
../../node_modules/d3-selection/src/selection/index.js -> ../../node_modules/d3-selection/src/selection/selectAll.js -> ../../node_modules/d3-selection/src/selection/index.js
../../node_modules/d3-selection/src/selection/index.js -> ../../node_modules/d3-selection/src/selection/filter.js -> ../../node_modules/d3-selection/src/selection/index.js
...and 12 more
created dist/all-components-bundle.js in 12.3s
```

If we introduce circular dependencies or add a library that introduces
additional circular dpendencies they may be missed.

## 👩‍💻 Implementation

Implemented a rollup onwarn handler that can filter out the d3 libraries
circular dependency warnings.

## 🧪 Testing

Validated locally that choosing a more specific prefix like
`d3-selection` will filter only warnings for that subset and let others
through.

## ✅ Checklist

- [x] I have updated the project documentation to reflect my changes or
determined no changes are needed.
  • Loading branch information
rajsite committed Sep 7, 2023
1 parent 3ec0d7b commit 4a306be
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 2 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "none",
"comment": "Filter rollup d3 related warning messages",
"packageName": "@ni/nimble-components",
"email": "rajsite@users.noreply.github.com",
"dependentChangeType": "none"
}
29 changes: 27 additions & 2 deletions packages/nimble-components/rollup.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,29 @@ const umdProductionPlugin = () => replace({
preventAssignment: true
});

// d3 has circular dependencies it won't remove:
// https://github.com/d3/d3-selection/issues/168
// So ignore just d3's circular dependencies following the pattern from:
// https://github.com/rollup/rollup/issues/1089#issuecomment-635564942
// Updated to use current onwarn api:
// https://rollupjs.org/configuration-options/#onwarn
const onwarn = (warning, defaultHandler) => {
const ignoredWarnings = [
{
code: 'CIRCULAR_DEPENDENCY',
file: 'node_modules/d3-'
}
];

if (
!ignoredWarnings.some(
({ code, file }) => warning.code === code && warning.message.includes(file)
)
) {
defaultHandler(warning);
}
};

// eslint-disable-next-line import/no-default-export
export default [
{
Expand All @@ -32,7 +55,8 @@ export default [
resolve(),
commonJS(),
json()
]
],
onwarn
},
{
input: 'dist/esm/all-components.js',
Expand All @@ -55,6 +79,7 @@ export default [
resolve(),
commonJS(),
json()
]
],
onwarn
}
];

0 comments on commit 4a306be

Please sign in to comment.