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

Incompatible with Webpack #5

Open
jeffijoe opened this issue Apr 7, 2016 · 10 comments
Open

Incompatible with Webpack #5

jeffijoe opened this issue Apr 7, 2016 · 10 comments

Comments

@jeffijoe
Copy link

jeffijoe commented Apr 7, 2016

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
@klardotsh
Copy link

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.

@damianobarbati
Copy link

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

@klardotsh
Copy link

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)

@damianobarbati
Copy link

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!

@percyhanna
Copy link

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

@maxkoryukov
Copy link

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.

@hsquek
Copy link

hsquek commented Oct 18, 2018

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.

@klardotsh
Copy link

klardotsh commented Oct 18, 2018

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). SpotOnInc@e760391 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 :)

@hsquek
Copy link

hsquek commented Oct 23, 2018

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

@markb-trustifi
Copy link

markb-trustifi commented Mar 31, 2022

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

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

No branches or pull requests

7 participants