🦋 Changeset detected

Latest commit: 5ffb038

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.02%. Comparing base (4efc038) to head (5ffb038).
Report is 2 commits behind head on main.

@@           Coverage Diff           @@
##             main    #1445   +/-   ##
=======================================
  Coverage   81.02%   81.02%           
=======================================
  Files          54       54           
  Lines        2213     2213           
  Branches      658      654    -4     
=======================================
  Hits         1793     1793           
  Misses        416      416           
  Partials        4        4           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

I might be doing something silly, but after checking the files, it doesn't look like @babel/runtime is used anywhere and in dist, so I think we can remove it.

This is a little bit surprising to me cause our compilation target is pretty low:

From what I can see this node version already supports things like object rest spread and classes so perhaps this is actually suspected. We don't use any fancy/esoteric new syntaxes that would usually require those helpers.

After fiddling with https://github.com/Thinkmill/manypkg today too, I think I understand better now. The babel config is probably missing plugins: ["@babel/plugin-transform-runtime"], which will use from @babel/runtime. So right now all the runtime helpers are inlined into the package instead.

So I think we have 2 directions here:

1. (this pr) remove @babel/runtime and inline the runtime helpers
2. Add @babel/plugin-transform-runtime to use the runtime package

I'm fine with either way. @babel/runtime isn't particularly heavy so I think it doesn't hurt to use it if we want. On the other hand, if I compile with the babel plugin, it looks like we only use 2 helpers from the dependency:

Screenshot

Tbf, im not that sure if Changesets would work today on such an old version of node. If we couod confirm that it doesnt… then we could bump the compilation target and drop this dependency

Tbf, im not that sure if Changesets would work today on such an old version of node. If we couod confirm that it doesnt… then we could bump the compilation target and drop this dependency

Since the same question came up in #1446, I've already investigated this a bit. There are 9 dependencies of changesets that specify they only support Node 10 in their engines field, so I expect changesets would not run on Node 8

At times, those engines declarations are not accurate/up to date. You could try to run changeset version && changeset publish on node 10 and node 12. There is non-zero chance that it would fail during init time or smth in one of them.

I haven't got the chance to test on node 8 and above since it's a hard to run them on macos arm (and nvm needs to compile them locally due to lack of prebuilt binaries). Maybe we can setup running CI on them to check it?

In any case, I think this PR wouldn't be too bad if merged too. The dependencies are already unused today.

The CI passes for me on Node 8, but running changeset version on an actual project doesn't:

$ npm run changeset:version

> kit-monorepo@0.0.1 changeset:version ~/src/kit
> changeset version

node_modules/globby/index.js:28
	} catch {
	        ^

SyntaxError: Unexpected token {
    at createScript (vm.js:80:10)
    at Object.runInThisContext (vm.js:139:10)
    at Module._compile (module.js:617:28)
    at Object.Module._extensions..js (module.js:664:10)
    at Module.load (module.js:566:32)
    at tryModuleLoad (module.js:506:12)
    at Function.Module._load (module.js:498:3)
    at Module.require (module.js:597:17)
    at require (internal/module.js:11:18)
    at Object.<anonymous> (node_modules/@manypkg/get-packages/dist/get-packages.cjs.dev.js:16:14)

It fails trying to run globby@11.1.0 which has:

  "engines": {
    "node": ">=10"
  }

Does it run on node 10? ;p

As far as I can tell, it does run on Node 10. I don't have a GitHub token setup locally in order to be able to test it the whole way through

I wonder if we should just remove babel? It doesn't really seem like it's necessary since object spreading has been fully supported since Node 8. This PR seems like a good first step

Node 10 doesn't support optional chaining syntax. I'd like to be able to use it :P