Change 412956 had a related patch set uploaded (by Zfilipin; owner: Zfilipin):
[mediawiki/core@master] WIP Update page object pattern in Selenium tests

https://gerrit.wikimedia.org/r/412956

As far as I can tell, import() is not supported yet even in the latest Node.js release. :|

https://nodejs.org/api/esm.html#esm_unsupported

wdio page object example code still uses ES5 syntax:

https://github.com/webdriverio/webdriverio/tree/master/examples/pageobject

Looks like it works, but Babel is required.

https://github.com/webdriverio/webdriverio/issues/1641#issuecomment-252897810

Example code: https://github.com/zeljkofilipin/webdriverio/commit/5c4a8503dc9a9494e329b10d50809263d579e1b9

How to test the above code:

git clone git@github.com:zeljkofilipin/webdriverio.git
cd webdriverio
npm install
npm run build
chromedriver --url-base=wd/hub --port=4444
./bin/wdio examples/pageobject/wdio.conf.js --spec examples/pageobject/specs/dynamic.spec.js

@WMDE-Fisch, @Ladsgroup and @Jdrewniak: I have updated task description. Any help is appreciated!

Thanks a lot to @Jdrewniak. From 412956:

Jdrewniak

Patch Set 3: Code-Review-1

I don't think you can use the ES6 exports together with the common.js require() . Instead you need to use ES6 imports as well.

The patch does works if you change the following in spec/user.js:

from (spec/user.js#lines 1-5):

 'use strict';
 const assert = require( 'assert' ),
	CreateAccountPage = require( '../pageobjects/createaccount.page' ),
	PreferencesPage = require( '../pageobjects/preferences.page' ),
	UserLoginPage = require( '../pageobjects/userlogin.page' );

to:

'use strict';
const assert = require( 'assert' );
import CreateAccountPage from '../pageobjects/createaccount.page';
import PreferencesPage from '../pageobjects/preferences.page';
import UserLoginPage from '../pageobjects/userlogin.page';

and the same for page/spec.js

I think 412956 is ready. It should not be merged until all extensions with Selenium tests are updated too, since this change will break tests in extensions. Before I update all extensions (7 at the moment), I would like to make sure my changes to package.json and adding .babelrc would not be reverted. 😅

I am looking at Developers/Maintainers#MediaWiki_core but it is not clear to me who should I ask for reviews. 🤔

To make it explicit: I think this is ready. I am just waiting for a review or two to let me know this is OK in general, before I update all extensions.

From 412956:

@Krinkle
Mar 22 5:07 AM
Patch Set 7: Code-Review-1

There should be no need to involve Babel here. Doing so is imho not wise as it makes it much more difficult to debug what code actually runs. We run on Node and we control the version, and it certainly allows us to use more native ES6 features than we can in browser code right now.

I think running without needing Babel would make the code easier to debug as well. The tests are relatively simple and straight forward.

In addition, unlike for browser code, use of import and other ESM stuff is still experimental. So we wouldn't just be emulating features of newer Node versions, we'd be using syntax that is unstable and not implemented by default in any version of Node, not even the latest current upstream version.

Secondly, these tests use WebDriver which provides the ability to run JS code in the browser. When writing tests in non-JS host languages, these are typically passed as strings. In JS hosts, it's the convention to write them as callbacks so they look like native code, however that isn't actually the case (the test framework just serialises the callbacks' statement to a string and sends it to eval in the browser). That code would be affected by Babel as well and could make for additionally complex situations.

Babel is a great framework for using latest browser features in older browsers, but I don't think that is an issue with our CI environment (if it is, we can always use a separate Docker image for this job that packages a newer version of Node). And either way, I think it's especially important for a test environment to have less indirection and complexity so that one may have higher confidence in its result.

I'm aware the Babel option is one of the upstream-documented ways to use WebdriverIO but it is by no means a requirement (couldn't be given you interact with the same package either way, and it doesn't know the difference). Upstream also documents ways to use WebdriverIO with TypeScript, but again, these are to show how to adopt it in that context if needed/preferred. I don't see it as a best practice or recommendation from them in general.

@Krinkle
Mar 22 5:07 AM
Patch Set 7:

(Node ESM: https://nodejs.org/api/esm.html )

@Krinkle
I'm aware the Babel option is one of the upstream-documented ways to use WebdriverIO but it is by no means a requirement (couldn't be given you interact with the same package either way, and it doesn't know the difference). Upstream also documents ways to use WebdriverIO with TypeScript, but again, these are to show how to adopt it in that context if needed/preferred. I don't see it as a best practice or recommendation from them in general.

As far as I can see, upstream documentation recommends two page object implementations, ES5 and ES6. Until recently ES6 implementation did not require Babel. Of course, we are free to continue using our current ES6 implementation, but then it would differ from upstream documentation. I would like to minimize the differences as much as possible. In this case, I think discussing this with upstream would be a good move.

@Krinkle could you please open an upstream issue explaining why you think Babel is not a good choice for page object implementation? I could copy/paste your comments, but I am sure there would be a discussion and I think you are far more likely to make a good case than I could.

@zeljkofilipin I don't think it's a problem for upstream.

Some projects use TypeScript, CoffeeScript, ES-latest, or ES-experimental with Babel. If you're working on such project, you can import any plain ES5 JS library from npm without a problem. Why? Because the packages on npmjs.org are (almost) always in plain ES5. And our hypothetical CoffeeScript project also runs as plain ES5 on Node.js. The "other" syntax is only in the Git of the locally run code, before compilation.

When writing code in ES-latest, and importing a module from npm, what matters is:

• At run-time your code must use require(), because that's how Node.js works.
• At run-time your code must use the public API of the library that you import.

If your project is not plain ES5 but something else, then instead of require(), you will write whatever thing your compiled language will change to require(). In the case of ES2016, that is import.

If a project uses Babel already, then developers usually configure their repository to automatically compile all JS files with Babel. So that if they get started with WebdriverIO, they'll probably use the Babel syntax by default, even if upstream doesn't document that. In fact, their ESLint will probably warn if they accidentally copied require(). Not because it is a problem (both works fine), but because it is a style preference.

• WebdriverIO is a library published to npmjs.org as plain ES5 code.
• Node.js natively runs plain ES5 and ES6 (but not ESM or "ES2016 modules"). This means the "real" code will always use require().

It is nice that WebdriverIO has ready-to-copy code snippets for multiple language variants. Easy to use. But mediawiki/core is not a Babel or ES-experimental project, and I see only negative value in compiling via Babel over a few superficial differences like import/export vs module/require.

I really appreciate your enthusiasm for perfection and to make our code the same as WebdriverIO recommends upstream, but... I think that coding style is not something we should take from upstream. When it comes to how we use whitespace, or whether we require semi-colon insertion, or whether we use Babel; that is a project choice. I do not think this is something upstream should recommend. I do not believe WebdriverIO wants all users to use Babel and use the same coding style rules as them.

Also, I do not think it would scale. For example, if the Mocha manual recommends CoffeeScript and different whitespace rules, we cannot do both? And we don't have to, because which whitespace rules we use and whether we use Babel, does not influence much how the code runs, and is of no concern for being supported by upstream.

I think it makes sense for us to use ES6 syntax for convenience (which is supported by Node.js and what we use in master now). But if you prefer choosing between ES5 or ESM-with-Babel, then I would recommend ES5.

Using ES5 also has the benefit of allowing us to use the same coding style in mediawiki/core for browser javascript, QUnit tests and Selenium tests; which will make contributions easier.

Thanks a lot @Krinkle, things are more clear to me now. I like our current ES6 syntax. I think ES5 syntax would be a step back, and Babel is an overkill. I will submit a patch request upstream adding our ES6 syntax as an option, in addition to their current ES5 and Babel syntax.

Change 412956 abandoned by Zfilipin:
WIP Selenium: Update page object pattern

https://gerrit.wikimedia.org/r/412956

This will be resolved by T373125: Use ESM in core for selenium tests.