Dynamically import locales

[AmauryD]

No description provided.

[AmauryD]

@RobbieTheWagner
The only downside of the initializer is that it always import the core of the flatpickr library. Which is fine most of the time because the core is imported when we use the component in the app at least once.
My only concern is when route splitting is enabled. The library will probably always be imported even if not used in the route.
In this case i could create a @embroider/macro configuration flag that disable the initializer import when requested.

[RobbieTheWagner]

Do we need this commented out code?

[RobbieTheWagner]

@RobbieTheWagner The only downside of the initializer is that it always import the core of the flatpickr library. Which is fine most of the time because the core is imported when we use the component in the app at least once. My only concern is when route splitting is enabled. The library will probably always be imported even if not used in the route. In this case i could create a @embroider/macro configuration flag that disable the initializer import when requested.

I would love to find a way to make things work without needing to force flatpickr onto the window so early. I admit, I still don't understand why we need it on the window for locales to work. I would expect importing the locale and then passing what you imported in to work.

[AmauryD]

I would love to find a way to make things work without needing to force flatpickr onto the window so early. I admit, I still don't understand why we need it on the window for locales to work. I would expect importing the locale and then passing what you imported in to work.

Me too, it was working before because the lib was put in vendor.js that was loaded at the start of the app. https://github.com/RobbieTheWagner/ember-flatpickr/blob/93d73ca7d0f501c68702730b708f2f0eca321ed1/index.js.

For the explanation, i'll try to make an example repo for a clearer explanation of the problem. My english is sometimes not the best. And flatpickr has a weird initialization logic.

I'll do some research on my side.

[AmauryD]

I've made an example repo with console.logs to see the problem.
https://github.com/AmauryD/demo-flapickbug

.

To be able to register a locale globaly, flatpickr must be registed before the locale file imported. Otherwise, the locale is never registered to the global window.flatpickr.l10ns. The problem is that ember calls the import 'flatpickr' after you import the locale. That moment is when the EmberFlapickr component is invoked. So the locale is never registered to the global object and can't be resolved using the string syntax.

I know it's a weird behavior but that's how flatpickr deals with locales.

Not sure if the explanation is better.

[AmauryD]

I added an option to disable this behavior. So it can be manually disabled and then tree-shaked if needed.
What do you think ?

ember-flatpickr/ember-flatpickr/src/initializers/ember-flatpickr.ts

Lines 4 to 13 in 91bbe64

	* This file is only here to initialize the flatpickr registry at the start of the app
	* This behavior can be disabled by setting `initializeFlatpickr` to `false` in the `@embroider/macros` config. The module will be then tree-shaken away if not used in the app.
	*/
	interface FlatpickrConfig {
	initializeFlatpickr: boolean;
	}
	if (macroCondition(getOwnConfig<FlatpickrConfig>()?.initializeFlatpickr)) {
	importSync('flatpickr');
	}

_{^{Will need a glass of whiskey after this}}

[AmauryD]

The merge from main broke the tests. I will take a look tomorrow

[RobbieTheWagner]

To be able to register a locale globaly, flatpickr must be registed before the locale file imported

What if the ember-flatpickr component took locale="fr" for example and then we handled the logic inside the component to dynamically import the locale files? At that point, the component should have already been instantiated and the flatpickr reference should exist.

[AmauryD]

To be able to register a locale globaly, flatpickr must be registed before the locale file imported

What if the ember-flatpickr component took locale="fr" for example and then we handled the logic inside the component to dynamically import the locale files? At that point, the component should have already been instantiated and the flatpickr reference should exist.

That's a possibility, but i don't know if the tree shaking will work if you dynamically import the locale based on runtime variables. Will try this later.

[AmauryD]

It looks like if i do something like this:
(Just a test code, not the final result)

  async setupFlatpickr(element: HTMLInputElement): Promise<void> {
    const { date, onChange, wrap, locale } = this.args;

    // Require that users pass a date
    assert(
      '<EmberFlatpickr> requires a `date` to be passed as the value for flatpickr.',
      date !== undefined,
    );

    // Require that users pass an onChange
    assert(
      '<EmberFlatpickr> requires an `onChange` action or null for no action.',
      onChange !== undefined,
    );

    // Wrap is not supported
    assert(
      '<EmberFlatpickr> does not support the wrap option. Please see documentation for an alternative.',
      wrap !== true,
    );

    if (locale) {
      await import(`flatpickr/dist/l10n/${locale}`);
    }


    // Pass all values and setup flatpickr
    scheduleOnce('afterRender', this, this._setFlatpickrOptions, element);
  }

It works. Webpack generates a bundle for each lang. These are lazy-loaded later. Great ! These bundlers are very smart.

All the small red squares are a locale.

When i choose a lang that is not already loaded it calls the chunk.

[RobbieTheWagner]

Great, glad that dynamic importing seems to work! Let's do that then, and not add an initializer.

[AmauryD]

Just a side note, but i tried to upgrade eslint-plugin-ember and there are some deprecations concerning using @ember/runloop and 'afterRender' queue.

[RobbieTheWagner]

@AmauryD can you please explain this waitFor here? I thought test waiters would be in test files or test helpers?

[AmauryD]

Due to the change of _setFlatpickrOptions to async, it caused issues with one test. I needed to add a waitFor statement in the suite. However, for user convenience, I preferred to add @waitFor from @ember/test-waiters to the addon code and do not change the test suite, as there's a chance that consumers' test suites might also break.

What do you think ?

[RobbieTheWagner]

@AmauryD I don't think application code should be handling cases for testing. If anything, we should add some of the waiting to the test helpers we ship, but I don't think we want to add waitFor to the addon code. I could be wrong, but that addon sounds like it is for test helpers.

[AmauryD]

I'm fine with it.
Adding the waiting logic to the current test-support would be great. But i currently don't know how i would check that the operation of the flatpickr component is done without service level code or some trickery.

[RobbieTheWagner]

This is great, thanks so much for taking this on!

[AmauryD]

No problem, thank's for your support !