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

fix: vite moment error #2307

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

akki-jat
Copy link

@akki-jat akki-jat commented Jan 24, 2023

It fixed issue when used in vite it shows localData is not a function error.

Also includes fix from #2228

@akki-jat
Copy link
Author

@dangrossman Please take a look.

@ghost
Copy link

ghost commented Feb 14, 2023

Would be very happy to see this PR approved soon @dangrossman, it would fix our builds and prevent us from looking for alternatives...

@dangrossman
Copy link
Owner

Would be very happy to see this PR approved soon @dangrossman, it would fix our builds and prevent us from looking for alternatives...

You're waiting on a 11 year old library with no updates in years to fix your build process? Merge the PR in your copy, or find an alternative. I have no idea what Vite is and have no way of knowing if this PR will break others' builds. Someone put require('moment') in there on purpose, now it's being changed to require('moment/moment'). That must break something, or it'd always have been moment/moment. I know moment isn't changing APIs as they archived that project years ago.

@Jogai
Copy link

Jogai commented Feb 15, 2023

At least set the project to archived then, to show visitors that its not under maintenance anymore.

@ghost
Copy link

ghost commented Feb 15, 2023

You have a point there @dangrossman. But it's just that I love the simplicity and integration with Bootstrap in this picker so a big #h5yr for that (and I so want to keep using this).

Now, JS is not my native language, but from what I've read here it shouldn't break anything, and sounds like a comparable issue:

When requireing a package in a js file, it will point to the main field in package.json, which includes name of the main file of the package and its relative address to package.json file. But it is possible to be extra specific about the file you wish to include and add the file name after package name, it's a bit overuse when package exports only one file, but useful when a package has multiple files with different functionalities to offer.
So using require('moment/moment') is more detailed way of using require('moment'), it should not break older usages.

@shravyaRB
Copy link

I was able to fix the error by defining an alias

alias: {
      moment: path.resolve(__dirname, 'node_modules/moment/moment.js'),
}

@clipsmm
Copy link

clipsmm commented Aug 9, 2024

I was able to fix the error by defining an alias

alias: {
      moment: path.resolve(__dirname, 'node_modules/moment/moment.js'),
}

Does this still work. Still getting the same error after this

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

Successfully merging this pull request may close these issues.

5 participants