Skip to content

chore: drop import-cwd#227

Merged
ai merged 2 commits intopostcss:mainfrom
43081j:drop-import-cwd
Jan 1, 2022
Merged

chore: drop import-cwd#227
ai merged 2 commits intopostcss:mainfrom
43081j:drop-import-cwd

Conversation

@43081j
Copy link
Copy Markdown
Contributor

@43081j 43081j commented Dec 31, 2021

@ai up to you if you want this or not ofc.

basically we introduced import-cwd at some point which is arguably fairly redundant since we can just use the built in node functions for this directly. personally don't see the point in these one-liner packages.

by dropping it, you can reduce the dependencies tree by at least 2-3 packages (surprisingly for something so simple).

let me know if you agree

Type

  • CI
  • Fix
  • Perf
  • Docs
  • Test
  • Chore
  • Style
  • Build
  • Feature
  • Refactor

SemVer

  • Fix (:label: Patch)
  • Feature (:label: Minor)
  • Breaking Change (:label: Major)

Checklist

  • Lint and unit tests pass with my changes
  • I have added tests that prove my fix is effective/works
  • I have added necessary documentation (if appropriate)
  • Any dependent changes are merged and published in downstream modules

@ai
Copy link
Copy Markdown
Member

ai commented Dec 31, 2021

This is my favorite type of PRs!

@ai
Copy link
Copy Markdown
Member

ai commented Dec 31, 2021

But we need to support old Node.js 10

@43081j
Copy link
Copy Markdown
Contributor Author

43081j commented Jan 1, 2022

hmm, the package we were depending on pretty much does this same code, so maybe we already regressed when we introduced it? (supporting node 10)

looking at the node docs, it looks like 10.x has createRequireFromPath rather than createRequire

@ai
Copy link
Copy Markdown
Member

ai commented Jan 1, 2022

Breaking changes is not worth for 2-3 dependencies reduction

@43081j
Copy link
Copy Markdown
Contributor Author

43081j commented Jan 1, 2022

its ok i've figured it out, we were using an older version of the dependency. ill see if i can update the code

@43081j
Copy link
Copy Markdown
Contributor Author

43081j commented Jan 1, 2022

@ai can you trigger CI? this seems to work locally on node 10

@ai ai merged commit d7287ea into postcss:main Jan 1, 2022
@ai
Copy link
Copy Markdown
Member

ai commented Jan 1, 2022

Thanks. Released in 3.1.1.

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