bublejs / buble

https://buble.surge.sh
MIT License
869 stars 67 forks source link

Bump magic-string #184

Closed curran closed 5 years ago

curran commented 5 years ago

Closes #183

curran commented 5 years ago

Oh no! CI is failing, but only for Node 4 and 6.

These are the failing tests:

    modules.js
      ✓ disallows import statement
      1) disallows export statement
      ✓ imports are ignored with `transforms.moduleImport === false`
      2) exports are ignored with `transforms.moduleExport === false`
      3) imports and exports are ignored with `transforms.modules === false`
      ✓ Supports anonymous functions as default export
      ✓ Supports anonymous classes as default export

  1) buble
       modules.js
         disallows export statement:
     SyntaxError: Export 'foo' is not defined (1:9)
  1 : export { foo };
               ^
      at null.pp$4.raise (node_modules/acorn/src/location.js:15:13)
      at null.pp$1.parseTopLevel (node_modules/acorn/src/statement.js:27:12)
      at null.parse (node_modules/acorn/src/state.js:92:17)
      at Function.parse (node_modules/acorn/src/state.js:112:37)
      at Object.transform (src/index.js:56:16)
      at /home/travis/build/Rich-Harris/buble/test/test.js:53:16
      at _tryBlock (assert.js:320:5)
      at _throws (assert.js:339:12)
      at Function.assert.throws (assert.js:369:3)
      at Context.<anonymous> (test/test.js:52:16)
  2) buble
       modules.js
         exports are ignored with `transforms.moduleExport === false`:
     SyntaxError: Export 'foo' is not defined (1:9)
  1 : export { foo };
               ^
      at null.pp$4.raise (node_modules/acorn/src/location.js:15:13)
      at null.pp$1.parseTopLevel (node_modules/acorn/src/statement.js:27:12)
      at null.parse (node_modules/acorn/src/state.js:92:17)
      at Function.parse (node_modules/acorn/src/state.js:112:37)
      at Object.transform (src/index.js:56:16)
      at Context.<anonymous> (test/test.js:57:16)
  3) buble
       modules.js
         imports and exports are ignored with `transforms.modules === false`:
     SyntaxError: Export 'foo' is not defined (1:23)
  1 : import 'foo'; export { foo };
                             ^
      at null.pp$4.raise (node_modules/acorn/src/location.js:15:13)
      at null.pp$1.parseTopLevel (node_modules/acorn/src/statement.js:27:12)
      at null.parse (node_modules/acorn/src/state.js:92:17)
      at Function.parse (node_modules/acorn/src/state.js:112:37)
      at Object.transform (src/index.js:56:16)
      at Context.<anonymous> (test/test.js:57:16)

@Rich-Harris Any insights here as to why magic-string version upgrade would induce these failing tests?

adrianheine commented 5 years ago

That's actually a change in acorn.

curran commented 5 years ago

Aha! Excellent insight! Thank you @adrianheine .

adrianheine commented 5 years ago

I just pushed 77ab2365788f7d2f0f05f79e2925e98b259404e0 to fix that.

curran commented 5 years ago

Closing in favor of https://github.com/Rich-Harris/buble/pull/185

adrianheine commented 5 years ago

I think this one could be merged much quicker than #185, see my comments there.

curran commented 5 years ago

@adrianheine Makes sense. I merged latest master, should be good to go.

adrianheine commented 5 years ago

Most of your changes to package-lock.json come from us using different npm or node versions, I guess. If I pull your branch and run npm i they are all reverted.

curran commented 5 years ago

Yes, I was a bit confused about those changes myself. I apologize if they cause problems.

These are the Node and NPM versions I'm on:

$ node --version
v11.9.0
$ npm --version
6.5.0

Perhaps best to use your generated version of package-lock, with fewer changes?

adrianheine commented 5 years ago
$ node --version
v10.15.1
$ npm --version
5.8.0

Yeah, keeping my version would be nice since I'm currently the person doing most of the commits in this project. Ideally a package-lock.json would not be so volatile, but well …

curran commented 5 years ago

Closing in favor of https://github.com/Rich-Harris/buble/pull/186 , as the commits here would make a mess in Git history.

adrianheine commented 5 years ago

btw, I'm absolutely willing to invest some work squashing otherwise good commits, and you could have used git add -p for picking only the parts of your changes we want to merge.

curran commented 5 years ago

Ah, good to know for future work. Thanks for the tips!