V2 Format about ember-inputmask HOT 18 CLOSED

Yes, should be an easy fix. Just letting you know was going to roll that in as well

from ember-inputmask.

I thought I could find a way around it, but appears that the only way to get the defaults to set once is an applicationInitializer. I really try to avoid them but thats what is for, initializing the application

from ember-inputmask.

Sure @cah-briangantzler you are always welcome. Please be my guest.

from ember-inputmask.

Im more use to yarn this looks like you prefer npm will try to do it that way, most my guides and examples use yarn though.

from ember-inputmask.

I'm totally down.

from ember-inputmask.

While updating to V2, the ember-modifier is being updated to 3.2 (previously on 3.1 I would assume). This means there are some deprecations showing in the console. I intend to fix them, not sure if it will be a breaking change or not. You should also read https://github.com/ember-modifier/ember-modifier/blob/v3.2.7/MIGRATIONS.md so you understand what/why the changes are happening

from ember-inputmask.

Yes I know. I've read that. new modify hook to rule them all 😄 but for this addon it shouldn't be a big problem.

from ember-inputmask.

A Few questions

How tied are you to the lazy import of inputmask at https://github.com/sinankeskin/ember-inputmask/blob/main/addon/modifiers/inputmask.js#L17.

Trying to write a couple tests and it sometimes fails. I dont think the settled of ember knows about the promise and sometimes the test asserts run before the inputmask is attached to the element

_setupInputMask https://github.com/sinankeskin/ember-inputmask/blob/main/addon/modifiers/inputmask.js#L57 takes an args statement which is passed when initialized at https://github.com/sinankeskin/ember-inputmask/blob/main/addon/modifiers/inputmask.js#L33, yet in the hook didUpdateArguments https://github.com/sinankeskin/ember-inputmask/blob/main/addon/modifiers/inputmask.js#L13 its called with no arguments, is this correct? Is didUpdateArguments even needed?

the method getArgs https://github.com/sinankeskin/ember-inputmask/blob/main/addon/modifiers/inputmask.js#L6 looks like it uses the named parameters, if there are none it uses the first positional or {}. Is this some backward compatibility thing that allows a hash of options as the first positional argument? I cant find anything in your docs/examples that refers to a positional parameter. Can this be removed and use only then named parameters?

from ember-inputmask.

Lazy import was a request by a user. This is kind of good if we have feature. I don't know can modify hook be async.

didUpdateArguments and related things all my mistakes. I don't think needed and can be removed.

getArgs things are intentional. We could use both as a component and as a modifier. If we want to use as a component args comes as first positional parameter to modifier and because of that I believe we should not remove this behaviour @cah-briangantzler

from ember-inputmask.

Ahh, I should have caught that on the args.

Will look and see if modify is async and ask about getting it so if it is not. Else I will have to put a wait in the tests which Im sure is a smell. When users write tests that use this, Im would assume they would run into the same issue

Update: Modify is not async, nor is it all the up to modifier manager :(

Can you recall the user requesting it and why (dynamic import)? Here is the docs on when to use it. I dont think any of them really apply and it cautions to use it only when necessary https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/import#dynamic_imports

from ember-inputmask.

Just to clarify a something, this means you do NOT support updating any of the args after the input field is initially created. Is that correct? I say this because of the error in didUpdateArguments and the fact that _setInputMask creates a NEW instance if it were to update. Creating a new instance means that if the user was using registerAPI, they now have access to an old instance. Might have been better to have sent your own API in registerAPI instead of the mask.

If I would have multiple input fields on the screen, doesnt this set the default over and over again? The lazy import is getting in the way of making that any better as well.

Do I read this correctly? and What is the course of action you would like to take?

from ember-inputmask.

Just to clarify a something, this means you do NOT support updating any of the args after the input field is initially created. Is that correct? I say this because of the error in didUpdateArguments and the fact that _setInputMask creates a NEW instance if it were to update. Creating a new instance means that if the user was using registerAPI, they now have access to an old instance. Might have been better to have sent your own API in registerAPI instead of the mask.

If I would have multiple input fields on the screen, doesnt this set the default over and over again? The lazy import is getting in the way of making that any better as well.

Do I read this correctly? and What is the course of action you would like to take?

You are right, my initial thought was "User should not update any args after the initialization" but second thought, this could be necessary. So, we need to make sure to invoke _setInputMask with the new args. I believe InputMask will destroy the old instance and adds a new one to element. I'm not totally sure though.

from ember-inputmask.

Ahh, I should have caught that on the args.

Will look and see if modify is async and ask about getting it so if it is not. Else I will have to put a wait in the tests which Im sure is a smell. When users write tests that use this, Im would assume they would run into the same issue

Update: Modify is not async, nor is it all the up to modifier manager :(

Can you recall the user requesting it and why (dynamic import)? Here is the docs on when to use it. I dont think any of them really apply and it cautions to use it only when necessary https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/import#dynamic_imports

We don't use browsers import here. This method comes from ember-auto-import addon I believe. But looks like works exactly the same.

So, bad news that modify cannot be async. User reason was not to expand the bundle size with this package at that time. And load only when necessary.

from ember-inputmask.

ember-auto-import analyzes the import for tree shaking, but pretty sure thats the browser one. Both the static and dynamic he talks about here https://github.com/ef4/ember-auto-import#dynamic-import as the browser ones.

Whats the direction on registerAPI? if a new instance is created the registerAPI has to be called again, which means it needs to move out of initialize which is a problem for the first time use. The register will be called with a null inputmask if the dynamic import is not completed. Code could get messy. Your call? Keep the dynamic import and try to deal with it, or make it static?

Keeping the dynamic import is going to cause some complexity with not setting the defaults over and over

from ember-inputmask.

ember-auto-import analyzes the import for tree shaking, but pretty sure thats the browser one. Both the static and dynamic he talks about here https://github.com/ef4/ember-auto-import#dynamic-import as the browser ones.

Whats the direction on registerAPI? if a new instance is created the registerAPI has to be called again, which means it needs to move out of initialize which is a problem for the first time use. The register will be called with a null inputmask if the dynamic import is not completed. Code could get messy. Your call? Keep the dynamic import and try to deal with it, or make it static?

Keeping the dynamic import is going to cause some complexity with not setting the defaults over and over

I agree. Let's get rid of the dynamic import for now. But to overcome the registerAPI issue instead of returning inputmask instance let's return element itself that way if element binds to another instance we could get new inputmask instance without too much effort. What do you think? @cah-briangantzler

from ember-inputmask.

I agree with that, but instead of returning the element, actually returning an API (that way we are not giving them acces to the element). Something like (not actual implementation, just an example)

    this.args.registerAPI.?({
         get inputmask = () => {
            return this.element.inputmask;
         } 
    });

This way you are returning an API with the property inputmask that dynamically looks it up. You would then be able to easily add more API if the need arises

This would be another breaking change as you are changing the signature of registerAPI

from ember-inputmask.

Even better. 👍🏻

from ember-inputmask.

Thank you again for the great upgrade 🙏

from ember-inputmask.