moment-round icon indicating copy to clipboard operation
moment-round copied to clipboard

Incompatible with Webpack

Open jeffijoe opened this issue 9 years ago • 10 comments

Hi, super awesome plugin!

It fails when compiled with webpack though:

WARNING in ./~/moment-round/dist/moment-round.js
Critical dependencies:
3:50-57 require function is used in a way in which dependencies cannot be statically extracted
 @ ./~/moment-round/dist/moment-round.js 3:50-57

jeffijoe avatar Apr 07 '16 12:04 jeffijoe

This is fixed in SpotOn's fork, which drops support for any loaders that are not CommonJS. Until the System Loader spec is finalized, this seems like a reasonable solution for Node/Browserify/Webpack folks.

klardotsh avatar May 23 '16 15:05 klardotsh

can't that fork be merged back here in the official plugin?

damianobarbati avatar Dec 30 '16 23:12 damianobarbati

We'd love to merge it upstream if it's wanted, but I kept it as a fork un-PR'd because I imagine it'd be impolite to PR such an invasive change (let alone one that breaks AMD support, for anyone still using that)

klardotsh avatar Dec 30 '16 23:12 klardotsh

Got it :)

But @iv597, I want to share with you my 2 cents: we as developers have the duty to move the development world forward. Backward compatibility does not mean accessibility: it means holding back innovation. It's because of this that we still have to deal with es5 and face countless headaches.

Anyway, thanks for this great moment plugin!

damianobarbati avatar Dec 30 '16 23:12 damianobarbati

Surely we could just use UMD to satisfy all environments for the time being, with a much less invasive change to the root package?

percyhanna avatar Apr 10 '17 21:04 percyhanna

Guys, maybe it is time to merge changes? WebPack still fails on build with this plugin, but this plugin is listed on the Docs page on moment.js

It is very disappointing when a public module doesn't work out of the box as expected. Such tradition drops shadow for entire NodeJS/JS ecosystem.

If somebody knows, how and what to merge - please, create a PR.

maxkoryukov avatar Oct 17 '17 19:10 maxkoryukov

Hi, I'm trying to use SpotOn's fork in my react application, but am having issues trying to set it up. Application is bundled with webpack. I can't seem to open an issue in his repo there so my apologies if I should not be posting here.

I thought the installation docs (I'm using npm) were too similar and so went with npm install --save-dev spotoninc-moment-round. I required the package

var moment = require('moment');
require('spotoninc-moment-round)

and got the following error:

m.round is not a function

I then decided to install the package as a full dependency and this was the error I encountered:

./node_modules/spotoninc-moment-round/dist/moment-round.js
Module build failed: Error: ENOENT: no such file or directory, open './node_modules/spotoninc-moment-round/dist/moment-round.js'
 @ ./src/Charts/BarChart.js 43:0-33
 @ ./src/Charts/index.js
 @ ./src/Store/AssetStore.js
 @ ./src/Store/index.js
 @ ./src/index.js
 @ multi (webpack)-dev-server/client?http://localhost:8080 webpack/hot/dev-server ./src/index.js
errors @ propTypes.js:3
onmessage @ propTypes.js:3
EventTarget.dispatchEvent @ propTypes.js:3
(anonymous) @ propTypes.js:3
SockJS._transportMessage @ propTypes.js:3
EventEmitter.emit @ propTypes.js:3
WebSocketTransport.ws.onmessage @ propTypes.js:3

I'm just starting out as a developer and would really appreciate any help I can get here. Thanks and my apologies again if this is not the place to be posting.

hsquek avatar Oct 18 '18 10:10 hsquek

It's been... a few years since I wrote that fork, and I've long since moved on from SpotOn, so I'm not in a position to really angle for that fork being merged (I think at this point @jimbattin is the probably best initial point of contact I know of about maintenance of that lib), but anyway, to address your specific issue, that was a documentation mismatch on my part back in the day (or more accurately, a failure to update the docs). https://github.com/SpotOnInc/moment-round/commit/e760391c8f2d25e8f313fbdb38b960f399259d6d added a monkey function that actually patches the moment object - this was I believe done due to some bizarre Webpack bug I was running into at the time?

thus you'll want to probably do

const moment = require('moment');
const moment_round = require('moment-round');

moment_round.monkey(moment);

Sorry I don't have better memory of why that workaround was added in the first place.

Ninja edit: wow, two years ago tomorrow I wrote that commit - nice timing :)

klardotsh avatar Oct 18 '18 23:10 klardotsh

Thanks! Appreciate it and will try it out soonish =)

hsquek avatar Oct 23 '18 02:10 hsquek

I'm using webpack with ES6:

import moment from 'moment';
import moment_round from 'moment-round';
let minTime= moment_round().add(5, 'minutes').ceil(5, 'minutes');

and receive an error:

Uncaught TypeError: Cannot read properties of undefined (reading 'fn')
    at Object.<anonymous> (moment-round.js:5:1)
    at Object../node_modules/moment-round/dist/moment-round.js (moment-round.js:55:2)
    at __webpack_require__ (bootstrap:19:1)
    at Module../app/js/controllers/myCtrl.ts (myCtrl.ts:1349:2)
    at __webpack_require__ (bootstrap:19:1)
    at node module decorator:5:1
    at index.js:241294:3
    at index.js:241296:12

markb-trustifi avatar Mar 31 '22 17:03 markb-trustifi