Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Circular dependencies inside d3 selection #168

Closed
Havunen opened this issue Feb 16, 2018 · 9 comments
Closed

Circular dependencies inside d3 selection #168

Havunen opened this issue Feb 16, 2018 · 9 comments

Comments

@Havunen
Copy link

Havunen commented Feb 16, 2018

When building D3 application using rollup it reports following circular deps.

Build started -- Fri Feb 16 2018 12:19:08 GMT+0200 (FLE Standard Time)
Circular dependency: node_modules\d3-selection\src\selection\index.js -> node_modules\d3-selection\src\selection\select.js -> node_modules\d3-selection\src\selection\index.js
Circular dependency: node_modules\d3-selection\src\selection\index.js -> node_modules\d3-selection\src\selection\selectAll.js -> node_modules\d3-selection\src\selection\index.js
Circular dependency: node_modules\d3-selection\src\selection\index.js -> node_modules\d3-selection\src\selection\filter.js -> node_modules\d3-selection\src\selection\index.js
Circular dependency: node_modules\d3-selection\src\selection\index.js -> node_modules\d3-selection\src\selection\data.js -> node_modules\d3-selection\src\selection\index.js
Circular dependency: node_modules\d3-selection\src\selection\index.js -> node_modules\d3-selection\src\selection\data.js -> node_modules\d3-selection\src\selection\enter.js -> node_modules\d3-selection\src\selection\index.js
Circular dependency: node_modules\d3-selection\src\selection\index.js -> node_modules\d3-selection\src\selection\exit.js -> node_modules\d3-selection\src\selection\index.js
Circular dependency: node_modules\d3-selection\src\selection\index.js -> node_modules\d3-selection\src\selection\merge.js -> node_modules\d3-selection\src\selection\index.js
Circular dependency: node_modules\d3-selection\src\selection\index.js -> node_modules\d3-selection\src\selection\sort.js -> node_modules\d3-selection\src\selection\index.js
Circular dependency: node_modules\d3-interpolate\src\value.js -> node_modules\d3-interpolate\src\array.js -> node_modules\d3-interpolate\src\value.js
Circular dependency: node_modules\d3-interpolate\src\value.js -> node_modules\d3-interpolate\src\object.js -> node_modules\d3-interpolate\src\value.js
Circular dependency: node_modules\d3-transition\src\transition\index.js -> node_modules\d3-transition\src\transition\filter.js -> node_modules\d3-transition\src\transition\index.js
Circular dependency: node_modules\d3-transition\src\transition\index.js -> node_modules\d3-transition\src\transition\merge.js -> node_modules\d3-transition\src\transition\index.js
Circular dependency: node_modules\d3-transition\src\transition\index.js -> node_modules\d3-transition\src\transition\select.js -> node_modules\d3-transition\src\transition\index.js
Circular dependency: node_modules\d3-transition\src\transition\index.js -> node_modules\d3-transition\src\transition\selectAll.js -> node_modules\d3-transition\src\transition\index.js
Circular dependency: node_modules\d3-transition\src\transition\index.js -> node_modules\d3-transition\src\transition\transition.js -> node_modules\d3-transition\src\transition\index.js
Circular dependency: node_modules\d3-voronoi\src\Diagram.js -> node_modules\d3-voronoi\src\Beach.js -> node_modules\d3-voronoi\src\Cell.js -> node_modules\d3-voronoi\src\Edge.js -> node_modules\d3-voronoi\src\Diagram.js
Circular dependency: node_modules\d3-voronoi\src\Diagram.js -> node_modules\d3-voronoi\src\Beach.js -> node_modules\d3-voronoi\src\Cell.js -> node_modules\d3-voronoi\src\Diagram.js
Circular dependency: node_modules\d3-voronoi\src\Diagram.js -> node_modules\d3-voronoi\src\Beach.js -> node_modules\d3-voronoi\src\Circle.js -> node_modules\d3-voronoi\src\Diagram.js
Circular dependency: node_modules\d3-voronoi\src\Diagram.js -> node_modules\d3-voronoi\src\Beach.js -> node_modules\d3-voronoi\src\Diagram.js
Bundle ready time: 170.151s  -- Fri Feb 16 2018 12:21:59 GMT+0200 (FLE Standard Time)

Rollup 0.56.0

@Klortho
Copy link

Klortho commented Apr 16, 2018

This isn't a problem with d3, and you can suppress this warning in rollup -- see rollup/rollup#1491 (comment).

@Havunen
Copy link
Author

Havunen commented Apr 16, 2018

I know the warning can be ignored, but isn't it bad practice to have circular dependencies within the D3 modules

@p-kuen
Copy link

p-kuen commented May 22, 2018

+1

@Klortho
Copy link

Klortho commented May 22, 2018

Is this unique to d3-flextree? Sorry, I guess I assumed that it was endemic to d3. Let me look at it....

@curran
Copy link
Contributor

curran commented Jun 28, 2018

I noticed this error as well.

Here is a bl.ock that reproduces it: Broken Rollup with D3-Selection - Circular Dependencies.

@mbostock
Copy link
Member

ES modules explicitly support cyclic dependencies and I have no current plans to address this warning from Rollup.

@curran
Copy link
Contributor

curran commented Nov 8, 2018

FWIW I figured out how to disable circular dependency warnings:

onwarn: function ( message ) {
  if (message.code === 'CIRCULAR_DEPENDENCY') {
    return;
  }
  console.error(message);
}

This goes at the top level of the configuration object in rollup.config.js.

@mhl
Copy link

mhl commented Jan 7, 2019

In case it's useful for anyone else coming across this, the helpful example from @curran above could be slightly improved by using the warn function passed in as the second parameter to the onwarn function, so the warning messages that are let through are formatted as they would be without this override, rather than writing the warning object with console.error:

  onwarn: function (warning, warn) {
    if (warning.code === 'CIRCULAR_DEPENDENCY') return;
    warn(warning);
}

(This is a link to the rollup documentation for owarn.)

severo added a commit to severo/pesticides_website that referenced this issue Mar 8, 2019
Razpudding added a commit to cross-cultural-data-literacy/2021-our-participants that referenced this issue Jul 27, 2021
flekschas added a commit to flekschas/regl-scatterplot that referenced this issue Jan 5, 2023
rajsite added a commit to ni/nimble that referenced this issue Sep 7, 2023
# 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.
@rajsite
Copy link

rajsite commented Mar 13, 2024

We wanted to suppress just warnings from d3 so we can catch our own regressions and audit other dependencies that are added so came up with the following onwarn handler:

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);
    }
};

Based on rollup/rollup#1089 (comment)

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

No branches or pull requests

7 participants