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

chore: drop import-cwd #227

Merged
merged 2 commits into from
Jan 1, 2022
Merged

chore: drop import-cwd #227

merged 2 commits into from
Jan 1, 2022

Conversation

43081j
Copy link
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
Member

ai commented Dec 31, 2021

This is my favorite type of PRs!

@ai
Copy link
Member

ai commented Dec 31, 2021

But we need to support old Node.js 10

@43081j
Copy link
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
Member

ai commented Jan 1, 2022

Breaking changes is not worth for 2-3 dependencies reduction

@43081j
Copy link
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
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
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