NPM Resource Changed ? Pending

User tests: Successful: Unsuccessful:

avatar C-Lodder
C-Lodder
9 May 2020

Summary of Changes

This PR reverts #28695 to retain support for IE11

Testing Instructions

Code review

avatar C-Lodder C-Lodder - open - 9 May 2020
avatar C-Lodder C-Lodder - change - 9 May 2020
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 9 May 2020
Category JavaScript Repository NPM Change
avatar C-Lodder C-Lodder - change - 9 May 2020
Labels Added: NPM Resource Changed ?
avatar HLeithner
HLeithner - comment - 9 May 2020

Isn't this the wrong approach? shouldn't we use a polyfill while creating the es5 files?

avatar dgrammatiko
dgrammatiko - comment - 9 May 2020

Only the subform and mutiselect are facing the front end, thus the rest can keep the newest methods

Isn't this the wrong approach? shouldn't we use a polyfill while creating the es5 files?

Yes you need a bundler for that. Also you need a bundler anyways because all NEW js is full of imports and somehow you need to resolve these before you serve those files...

avatar HLeithner
HLeithner - comment - 9 May 2020

So our build system needs an update? Can you do this?

avatar dgrammatiko
dgrammatiko - comment - 9 May 2020

So our build system needs an update? Can you do this?

Yup, fwiw it was one of my mistakes to not include a bundler for the JS when I did the last update. Are you ok with Rollup? (it's the industry standard these days, everybody moves away from Webpack)

avatar C-Lodder C-Lodder - change - 9 May 2020
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2020-05-09 09:13:54
Closed_By C-Lodder
avatar C-Lodder C-Lodder - close - 9 May 2020
avatar C-Lodder
C-Lodder - comment - 9 May 2020

@dgrammatiko If you're going to use rollup, you may aswell convert the com_media build scripts. No point in keeping webpack if your moving to rollup.

Will close this so a bundler can be used with a polyfill

avatar dgrammatiko
dgrammatiko - comment - 9 May 2020

@C-Lodder this is quite a task (introducing Rollup) and won't do anything unless I'm assured that this will be accepted

avatar HLeithner
HLeithner - comment - 9 May 2020

That's more a question to @wilsonge then to me

avatar dgrammatiko
dgrammatiko - comment - 9 May 2020

@wilsonge so basically there are 3 questions here:

  • Are you willing to accept a PR introducing Rollup as the JS bundler
  • Replace Webpack with Rollup for the media manager
  • Use the browsers native feature detection (type=module/nomodule) (given that the implementation will not introduce B/C breaks in any way)
avatar wilsonge
wilsonge - comment - 9 May 2020

So my experience isn't that everyone is moving from webpack to rollup. My experience is in the case of SPA's (increasingly common) people use rollup. In the case of many individual files webpack is still pretty dominant. I'm still not really seeing many websites who are using imports in production frontend code. Having said that I have nothing against rollup - I just think your exaggerate a bit on the usage.

Yes you need a bundler for that. Also you need a bundler anyways because all NEW js is full of imports and somehow you need to resolve these before you serve those files...

So babel will transpile all this stuff. It in theory should even transport the imports too. What I don't know/understand is what the benefit of rollup over babel transpiling is

avatar dgrammatiko
dgrammatiko - comment - 9 May 2020

I just think your exaggerate a bit on the usage.

Sure, I mean Vue switched to Rollup (hint the Webpack author is a Vue core contributor):
https://github.com/vuejs/vite/blob/6d2fabe7ca547eae7a9a0cc402ccb36b3b63adb2/package.json#L84

So babel will transpile all this stuff.

Yes Babel will transpile but AFAIK will not resolve the imports (or will not resolve from the node_modules, not sure). But even if that's the case Rollup is a nice, fully documented bundler that has plenty of plugins or even if a plugin doesn't exist writing it is way easier than figuring out why a Webpack configuration doesn't exactly do what you expected it to do.

I'm still not really seeing many websites who are using imports in production frontend code.

You're right the reason is that it's a performance killer but that's not what I'm suggesting because I don't have a solution that will fit the Joomla prerequisites (eg placing a file in the template's js folder and resolve the import from there and not from the media folder). FWIW when we started the ES6 project @Yves @dneukirchen and me had some lengthy discussions on how to handle that (the idea was to have a custom implementation of dynamic imports and some store with the overrides) but we never coded anything. Maybe there is a misunderstanding about modules here:
yes the files can have imports but nobody serves these files everybody resolve the import (concatenate the files, that's what it's all about) and also Joomla shouldn't serve js files with imports. So if the modules are bundled what's the difference from plain, non module files?
couple of things:

  • modules are deferred, which is the proper way to serve js
  • they are scoped, eg things are not leaking as globals (unless you do window.Joomla ={})

In short type=module doesn't mean that Joomla will start serving files with a bunch of imports, thus the need for the bundler

avatar Fedik
Fedik - comment - 9 May 2020

if it need only in these 2-3 files, just merge @C-Lodder suggestion, and move on ?

avatar dgrammatiko
dgrammatiko - comment - 9 May 2020

@Fedik yes you can do that but:

  • always serving a polyfill (in most browsers will be useless override of a browser's native functionality)
  • still have a bastard implementation on the JS, yes ES6+ is the proposed way of contributing code but that comes handicapped: no imports means these are not actually ES6+ files... (I mean take a look at the build tools and tell me how that would look like if we didn't used require() there ?)
avatar Fedik
Fedik - comment - 9 May 2020

in most browsers will be useless override of a browser's native functionality

A correct polyfill does not override existing functionality.

...and tell me how that would look like if we didn't used require() there

Well there are difference between import() on server side and client side. My state is that "modules" in current implementation useful only for server side, client side may have some benefits only with dinamyc import() , in far future, in galaxy not far away ?

I am understood your concern, but I would not bother much with this 3 files ?

avatar dgrammatiko
dgrammatiko - comment - 9 May 2020

but I would not bother much with this 3 files

The discussion is not about those 3 files but a general observation that we're not actually support properly ES6+ files..
Also I never said that I'm pro serving files with imports in the browser, I'm against, thus the whole topic about the bundler

Add a Comment

Login with GitHub to post a comment