[Glimmer2] Implement input

[asakusuma]

I don't think the late bound stuff can help with implementing input since lateBound gets called after CurlyComponentManager.create.

cc @krisselden @chadhietala @GavinJoyce

[chadhietala]

Seems like this should be it's own branch?

[asakusuma]

Sure. It would just mean always doing the lookup up-front which seems fine.

[asakusuma]

For whatever reason, one of the input tests is failing in Safari and Microsoft browsers. I'm looking into it.

[asakusuma]

I think saucelabs is having problems

Error: 07 Jun 05:09:02 - All jobs using tunnel have finished.
07 Jun 05:09:02 - Sauce Connect could not establish a connection.
07 Jun 05:09:02 - Please check your firewall and proxy settings.

[asakusuma]

I've changed the approach to re-write the AST. Any ideas why travis doesn't seem to be running?

[rwjblue]

wat

[rwjblue]

Please add loc information to these (builders.sexpr and builders.path).

[rwjblue]

What happens if -input-type is already specified?

[asakusuma]

@rwjblue how do I add loc information here? Neither buildSexpr nor buildPath accept a loc arguments like some of the other builders. Does that need to be added?

I don't think end-users can add a -input-type helper since it's registered as an internal helper. But if one already existed, I believe DynamicComponentSyntax only looks at the first argument, so if -input-type was already specified, it would be ignored.

[rwjblue]

Neither buildSexpr nor buildPath accept a loc arguments like some of the other builders. Does that need to be added?

Yes. Each AST node should be able to be built with a loc.

I don't think end-users can add a -input-type helper since it's registered as an internal helper.

There is nothing stopping a user from having already defined a -input-type helper in their application, but I'm not sure we should care about that too much...

I believe DynamicComponentSyntax only looks at the first argument, so if -input-type was already specified, it would be ignored.

If we expect this to go into exactly node.params[0] we should do that, and error if there was already something there.

For example today this "works" (doesn't blow up):

{{input "blah blah blah" value=someThing}}

If we are going to add (internally) significance to the first ordered argument to {{input, then we should have a helpful assertion if the user puts something there...

[asakusuma]

@rwjblue I meant that the end-user cannot use -input-helper unless they themselves define -input-helper. I confirmed this locally. My understanding is that a leading dash is reserved for internal helpers.

EDIT: In other words, the -input-helper I am introducing is not available for end users. They would have to make their own, which I'm under the impression is not supposed to happen given the convention.

I can certainly add a check to blow up if there's any pre-existing positional params. In other words, we are now saying that you cannot use positional parameters with {{input}}.

[rwjblue]

Looking good!

[asakusuma]

Working on figuring out why tests fail in Safari and MS browsers. Safari is failing for a different reason than MS.

[asakusuma]

Modifying input.selectionStart, which is utilized in the cursor tests, causes an event in Safari (does not happen in Chrome). This event kicks off a chain of events that results in an error because the view does not have sendAction implemented. Not sure if this is the right approach, but I solved this by turning off the event dispatcher for the input tests, since we are not interested in the event dispatcher for these tests.

[chadhietala]

@asakusuma Yea we need to land #13532.

[homu]

☔ The latest upstream changes (presumably #13492) made this pull request unmergeable. Please resolve the merge conflicts.

[krisselden]

@asakusuma Should you skip the failing test for now and we can recheck after sendAction?

[asakusuma]

I'm fine doing that, though I'm not sure sendAction is what's causing the Microsoft browser issues.

All the tests pass in Chrome, Safari, and Firefox. In Microsoft browsers, the null values test fails. What's failing is that when you set the value of the input to null, the value does not become an empty string in MS browsers for whatever reason.

[homu]

☔ The latest upstream changes (presumably #13684) made this pull request unmergeable. Please resolve the merge conflicts.

[asakusuma]

Discovered the source of the problem in MS Edge. removeAttribute('value'); doesn't seem to work in Edge. When the value needs to be null, glimmer uses removeAttribute('value'); to update the input element.

Here's a simple repro: https://gist.github.com/asakusuma/aad85f430f465903e56feee2b53a414c

cc @jdalton

[rwjblue]

@asakusuma - Have you reported an issue against MS Edge?

[asakusuma]

@jdalton in MS Edge, value is still "I have the best inputs" even after removeAttribute is run. On chrome, value attribute is removed and the input is empty.

I have not yet filed a ticket.

[homu]

☔ The latest upstream changes (presumably #13649) made this pull request unmergeable. Please resolve the merge conflicts.

[jdalton]

@asakusuma I filed a ticket internally.

[rwjblue]

@jdalton - Thank you!

[jdalton]

I've gotten removeAttribute to work for other attributes on other elements in Edge.
Do you all know of any other problem attributes?

[asakusuma]

@jdalton thanks! I'm not aware of any other problem attributes.

[jdalton]

Just a heads up, while making a feature detection for this I noticed that in Chrome:

var input = document.createElement('input');
var buggyRemoveAttrValue = !!(input.value=1,input.removeAttribute('value'),input.value);
buggyRemoveAttrValue;
// => true in Chrome and Edge

but

var div = document.createElement('div');
div.innerHTML = '<input value=1>';
var input = div.firstChild;

var buggyRemoveAttrValue = !!(input.removeAttribute('value'),input.getAttribute('value'));
buggyRemoveAttrValue;
// => false in Chrome but true in Edge

It's the old source attribute vs. element property thing.

[homu]

☔ The latest upstream changes (presumably #13725) made this pull request unmergeable. Please resolve the merge conflicts.

[asakusuma]

Did some bisecting, and it would appear that glimmerjs/glimmer-vm@951cbe5 introduced an incompatibility with the ember build.

Could not require 'ember-cli-build.js': Cannot find module 'glimmer-engine/ember-cli-build'
Error: Could not require 'ember-cli-build.js': Cannot find module 'glimmer-engine/ember-cli-build'
    at Function.Module._resolveFilename (module.js:339:15)
    at Function.Module._load (module.js:290:25)
    at Module.require (module.js:367:17)
    at require (internal/module.js:20:19)
    at Object.<anonymous> (/home/travis/build/emberjs/ember.js/ember-cli-build.js:45:21)
    at Module._compile (module.js:413:34)
    at Object.Module._extensions..js (module.js:422:10)
    at Module.load (module.js:357:32)
    at Function.Module._load (module.js:314:12)
    at Module.require (module.js:367:17)
    at require (internal/module.js:20:19)
    at module.exports (/home/travis/build/emberjs/ember.js/node_modules/ember-cli/lib/utilities/find-build-file.js:19:17)
    at CoreObject.module.exports.Task.extend.setupBroccoliBuilder (/home/travis/build/emberjs/ember.js/node_modules/ember-cli/lib/models/builder.js:48:21)
    at CoreObject.module.exports.Task.extend.init (/home/travis/build/emberjs/ember.js/node_modules/ember-cli/lib/models/builder.js:89:10)
    at CoreObject.superWrapper [as init] (/home/travis/build/emberjs/ember.js/node_modules/core-object/lib/assign-properties.js:32:18)
    at CoreObject.Class (/home/travis/build/emberjs/ember.js/node_modules/core-object/core-object.js:32:33)

cc @tomdale @rwjblue

[tomdale]

@asakusuma I think this is related to glimmerjs/glimmer-vm#190, we need to ship packages/ and ember-cli-build.js with the npm package for use by Ember.

[asakusuma]

After including the glimmer fix, Travis is now failing with a new problem

The Broccoli Plugin: [Funnel] failed with:
Error: ENOENT: no such file or directory, lstat '/home/travis/build/emberjs/ember.js/node_modules/glimmer-engine/node_modules/simple-html-tokenizer/lib'

[asakusuma]

@rwjblue do we need to increase the timeout?

Blueprints is also failing but I'm not seeing a useful error.

No output has been received in the last 10m0s, this potentially indicates a stalled build or something wrong with the build itself.

[asakusuma]

npm is still borking on the blueprints job. it keeps doing this which leads me to believe there is something wrong, but there's no useful error, just npm ERR! code 1

[rwjblue]

Cleared cache and restarted builds.

[rwjblue]

Thank you @asakusuma for sticking with this so long!