Skip to content

Fix circular dependencies#1321

Merged
ai merged 3 commits intopostcss:osefrom
mapgrid:fixes/ose-circular-dependencies
Jan 27, 2020
Merged

Fix circular dependencies#1321
ai merged 3 commits intopostcss:osefrom
mapgrid:fixes/ose-circular-dependencies

Conversation

@mapgrid
Copy link
Copy Markdown
Contributor

@mapgrid mapgrid commented Jan 19, 2020

While building a recent project with Rollup, we ran in to issues with circular dependencies in PostCSS. This PR solves circular dependencies across 3 files, and adds consistency to how files are included.

A sampling of current circular dependencies

./container.js -> ./rule.js -> ./container.js
./container.js -> ./parse.js -> ./parser.js -> ./rule.js -> ./container.js
./css-syntax-error.js -> terminal-highlight.js -> input.js -> css-syntax-error.js
./parse.js -> parser.js -> at-rule.js -> container.js -> parse.js
...

container.js

In container.js there are 3 late-style require in the normalize function. This PR removes the require completely and replaces it with a register dependency pattern.

root.js

In root.js there are 2 late-style require in the toResult function. This PR removes the require completely and replaces it with a register dependency pattern.

terminal-highlight.es6

In terminal-highlight.js there is 1 late-style require in the terminalHighlight function. This PR removes the require completely and replaces it with a register dependency pattern.

Improvement

  • All code is included at the top level of the file or by registering a dependency
  • 6 require statements removed
  • No public API is changed
  • PostCSS can be bundled with Rollup or other bundlers

Comment thread lib/container.js Outdated
Comment thread test/root.test.js
Comment thread lib/css-syntax-error.js Outdated
Comment thread lib/terminal-highlight.js Outdated
@ai ai merged commit cc5a017 into postcss:ose Jan 27, 2020
@ai
Copy link
Copy Markdown
Member

ai commented Jan 27, 2020

Great work! Do you have Twitter handle to mention you on our Twitter?

ai pushed a commit that referenced this pull request Jan 27, 2020
* fix circular dependencies

* move to static methods to fix imports

* remove default import from terminal highlight
@mapgrid mapgrid deleted the fixes/ose-circular-dependencies branch January 31, 2020 14:57
@mapgrid
Copy link
Copy Markdown
Contributor Author

mapgrid commented Jan 31, 2020

Thanks! Yes. Brand new account actually, getting ready to launch this new initiative: @mapgridorg

ai pushed a commit that referenced this pull request Feb 8, 2020
* fix circular dependencies

* move to static methods to fix imports

* remove default import from terminal highlight
ai pushed a commit that referenced this pull request Mar 28, 2020
* fix circular dependencies

* move to static methods to fix imports

* remove default import from terminal highlight
ai pushed a commit that referenced this pull request Mar 30, 2020
* fix circular dependencies

* move to static methods to fix imports

* remove default import from terminal highlight
ai pushed a commit that referenced this pull request Apr 8, 2020
* fix circular dependencies

* move to static methods to fix imports

* remove default import from terminal highlight
ai pushed a commit that referenced this pull request May 4, 2020
* fix circular dependencies

* move to static methods to fix imports

* remove default import from terminal highlight
ai pushed a commit that referenced this pull request May 29, 2020
* fix circular dependencies

* move to static methods to fix imports

* remove default import from terminal highlight
ai pushed a commit that referenced this pull request Jun 2, 2020
* fix circular dependencies

* move to static methods to fix imports

* remove default import from terminal highlight
ai pushed a commit that referenced this pull request Jun 21, 2020
* fix circular dependencies

* move to static methods to fix imports

* remove default import from terminal highlight
@ai ai added this to the 8.0 milestone Jul 14, 2020
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