Skip to content

Experimental datepicker example #3

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jzaefferer
Copy link
Owner

This is adding the datepicker rewrite, in its current state, to the demo, using the globalize-webpack-plugin. Currently this doesn't work, since datepicker.js and calendar.js specify two dependencies that can't be resolved, globalize/date and globalize-locales. To reproduce locally, check out this branch, run npm install and npm start:

ERROR in ./~/jquery-ui/ui/widgets/datepicker.js
Module not found: Error: Cannot resolve module 'globalize/date' in /[...]/webpack-jquery-ui/node_modules/jquery-ui/ui/widgets
 @ ./~/jquery-ui/ui/widgets/datepicker.js 24:2-33:14

ERROR in ./~/jquery-ui/ui/widgets/calendar.js
Module not found: Error: Cannot resolve module 'globalize/date' in /[...]/webpack-jquery-ui/node_modules/jquery-ui/ui/widgets
 @ ./~/jquery-ui/ui/widgets/calendar.js 23:2-36:14

ERROR in ./~/jquery-ui/ui/widgets/calendar.js
Module not found: Error: Cannot resolve module 'globalize-locales' in /[...]/webpack-jquery-ui/node_modules/jquery-ui/ui/widgets
 @ ./~/jquery-ui/ui/widgets/calendar.js 23:2-36:14

I looked at the mappings for these in jQuery UI's bootstrap.js, but couldn't figure out how these would/should work in the context of a webpack app.

Ping @rxaviers @fnagel @scottgonzalez - any input on this?

@fnagel
Copy link

fnagel commented Nov 20, 2016

Sorry, I have hardly to no experience with webpack.

@jzaefferer
Copy link
Owner Author

Can you shed some light on how the globalize-locales module is created, and how we expect endusers to deal with that, be it with AMD or webpack? That would already help a lot.

@fnagel
Copy link

fnagel commented Nov 20, 2016

@fnagel
Copy link

fnagel commented Nov 20, 2016

Afair we wanted to deliver a minimal (EN only) version while having some tool similar (or built-in) to download builder. Some Grunt tasks would be nice to I guess.

From the Datepicker wiki:

Currently, external/localization.js holds CLDR data. We need to figure out how to deal with this data in demos and tests.

@jzaefferer
Copy link
Owner Author

None of that should be necessary with the globalize webpack plugin, since that deals with loading and compiling CLDR data. I tried to simply ignored those dependencies using externals in the webpack config:

externals: {
	'globalize-locales': 'var {}',
	'globalize/date': 'var {}'
},

That produces a build, but it then fails at runtime due to missing CLDR data, "E_MISSING_CLDR: Missing required CLDR content supplemental/likelySubtags."

@rxaviers any idea why this isn't working?

@jzaefferer
Copy link
Owner Author

I did another PR, #4, where I more closely copy the webpack reference config from https://github.com/globalizejs/globalize/blob/master/examples/app-npm-webpack/webpack-config.js

@rxaviers
Copy link

Basically the issues are:

  • Globalize webpack plugin needs AMD support (or jQuery UI needs CJS exports)... The jQuery UI widgets have exports for AMD and globals, but not for CJS. The plugin doesn't "find" globalize imports via AMD currently https://github.com/rxaviers/globalize-webpack-plugin/blob/master/DevelopmentModePlugin.js#L45. A solution would be either a change in jQuery UI (add CJS export), or in the plugin (add AMD support).
  • Static parser of the Globalize compiler doesn't "find" globalize instances... The datepicker has a locale option, which it uses to generate the formatters. Such code isn't supported by the static parser, which currently just supports static calls like Globalize. using the globalize default locale.

@jzaefferer
Copy link
Owner Author

A solution would be either a change in jQuery UI (add CJS export), or in the plugin (add AMD support).

This seems like it would be much more useful to change in the plugin. Its really hard to tell how difficult that really is, but at a glance it doesn't seem so bad...

Static parser of the Globalize compiler doesn't "find" globalize instances... The datepicker has a locale option, which it uses to generate the formatters. Such code isn't supported by the static parser, which currently just supports static calls like Globalize. using the globalize default locale.

Here I wonder why we even have the locale option in calendar; without it, we could use the Globalize methods directly. @fnagel @scottgonzalez do you remember why we decided to add it? Looking at the wiki is rather inconclusive:

Need to add a culture/locale option to match spinner or decide to remove it from spinner.

As far as I remember, we wanted to remove that option from spinner...

@scottgonzalez
Copy link

As far as I remember, we wanted to remove that option from spinner...

Yup, that's probably just out-of-date information. We are definitely dropping the locale option from spinner in favor of providing custom parser and formatter options. However, calendar/datepicker will have a lot of options to set based on locale. We probably need to have a team discussion about how to handle 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.

4 participants