fgnass / spin.js

A spinning activity indicator
http://spin.js.org
MIT License
9.3k stars 1.02k forks source link

A warning during bundling with rollup #351

Closed paulsmirnov closed 6 years ago

paulsmirnov commented 6 years ago

The version 3.1.0 is distributed as an ES6 module, however, the module seems to use invalid syntax.

node_modules\spin.js\spin.js (1:16) The 'this' keyword is equivalent to 'undefined' at the top level of an ES module, and has been rewritten

The code which causes the trouble is located at the top. It looks like a polyfill to be used in the global context, not in ES6 module:

var __assign = (this && this.__assign) || Object.assign || function(t) {

Is there any way to correctly setup TypeScript compiler so that it produces valid ES6 modules for distribution?

davidlmorris commented 6 years ago

Given that ES6 support is patchy, it would seem that an ES5 distribution would make more sense at the moment.

paulsmirnov commented 6 years ago

@davidlmorris Right, that was another concern deserving a separate issue. Even if one distributes ES6 module, it is usually done in addition to ES5 one. The "main" field of package.json should still point to a CommonJS or UMD module, as there is a special "module" field for ES6 module distribution. They both still use ES5 syntax though. That's what I've seen in most other projects.

paulsmirnov commented 6 years ago

As for the original issue, there's a way to switch rollup warnings off (rollup/rollup#794) but it is not quite usable because:

So I'm still looking for a better way to resolve the issue.

vladshcherbin commented 6 years ago

@paulsmirnov hey, I'm also experiencing rollup + ts pain points. Here's what I found:

turning them off just for spin.js is possible with additional IFs but still we loose the default behavior for all other warnings (rollup/rollup#1245).

This solution works for me like mentined in this PR:

onwarn: (warning, warn) => {
  if (warning.code === 'THIS_IS_UNDEFINED') {
    return
  }

  warn(warning)
}

Although it doesn't solve the source of this warnings, but it's a workaround for now. Would be great to find a way to solve this warnings completely, without this hacks.

paulsmirnov commented 6 years ago

@VladShcherbin really? It didn't work for me, as warn === undefined. Probably, an issue with different versions. I thought the PR was not fully implemented/merged yet. Thanks for pointing this out, I should double check this solution.

paulsmirnov commented 6 years ago

@VladShcherbin thanks again, rollup 0.55.1 did the trick so I switched the warning off just for spin.js. I think I'll close this issue then, as the project doesn't seem to be maintained in a timely manner ☹️