convert to es6 modules #2108

patrickkettner · 2016-10-17T22:04:33Z

TODO BEFORE MERGE

add uglification to build step (minify is a noop currently)
get travis working on pull requests
update travis to include node 4 and 6
update import paths to be true file paths
get npm prepublish script working to flatten deps so folks can require('modernizr/emoji')
get test suite on travis running smoothly (hanging on Edge)
up to date README

rejas · 2018-07-04T08:37:04Z

are you still working on that? looks like it probably needs a rebase :-)

niksy · 2018-08-08T06:40:54Z

Another reason why having ES modules would be great: having option to opt-in for global namespace pollution (see #2362)

niksy · 2019-01-25T13:29:08Z

If anyone is interested in ES Modules implementation of current codebase, I’ve done it in my fork: https://github.com/niksy/modernizr-esm

There are still tests to be modified and written!

@patrickkettner @rejas let me know if this looks good to you!

rejas · 2019-01-30T08:42:12Z

hi @niksy to be honest I didnt look at that or have given much thought baout converting to ES6. Mainly due to even @patrickkettner not showing up anymore from all the maintainers. I would love to get some PRs reviewed and maybe even a new version out but with so much stuff in the testing pipeline broken right now which I have no clue to fix (bnor any help), this is too much for me to worry about. Maybe someone of the original maintainers shows up.
In the meantime I was thinking of starting a "modernizr-next" fork, going from the master branch here and just releasing it on a monthly basis for people that needs some of the stuff from after v3.6. Totally untested and without guarantees (like any -next fork :-) and not with the cool modenrizr.com build website but it might be better than nothing.

niksy · 2019-01-30T09:16:59Z

@rejas yeah, I completely understand. It’s unfortunate that Modernizr is now in this state, but we do what we can do. If you go with "modernizr-next", I would gladly switch dependancy to the fork and maintain ESM version of new features!

rejas · 2019-02-19T22:43:37Z

So I just published https://www.npmjs.com/package/modernizr-next if you want to check it out @niksy

patrickkettner · 2020-04-24T06:37:06Z

@rejas took ~3.5 years, but here is a esm rewrite

I also rebuilt the site from the ground up, it can go live once this gets published.

thank you for all of your work.

niksy · 2020-04-24T07:38:36Z

@patrickkettner anything particularly you found different in the approach I used in https://github.com/niksy/modernizr-esm?

patrickkettner · 2020-04-24T07:44:07Z

hi @niksy!

I made a number of changes, mostly just code cleanups/modularizations. I think the only thing of concern in modernizr-esm was you are calling testRunner on every addTest call, which logarithmically slows down the script every test you add - ex. the full build that is used in the test suite, it was crashing the page

niksy · 2020-04-24T07:52:01Z

Oh, yeah, I see. It should probably filter only that particular test added with addTest.

Did you find my approach viable for ESM version of Modernizr?

I see that you don’t use named exports for features with multiple tests (e.g. video). Any reason for that?

Maybe we can join efforts and I can deprecate my package in favor of this?

patrickkettner · 2020-04-24T08:11:16Z

I see that you don’t use named exports for features with multiple tests (e.g. video). Any reason for that?

oversight, I think. ill have to double check

Maybe we can join efforts and I can deprecate my package in favor of this?

im more than open to it, but would want @rejas feedback

rejas · 2020-04-27T20:52:28Z

Looked at around 300 of the files already :-) What I did notice is that the test pages are not working currently. So when i do "npm run serve-gh-pages" and open

http://localhost:8080/test/unit.html -> js errors, doesnt complete like https://modernizr.github.io/Modernizr/test/unit.html does

http://localhost:8080/test/integration.html -> no errors but doesnt list all the tests. it counts only 692 while the v3 branch on https://modernizr.github.io/Modernizr/test/integration.html has more than 780.... first glance shows that nested tests lie audio.ogg dont appear

rejas · 2020-04-27T20:53:01Z

Hoping to finish my review tomorrow. Nightinight!

patrickkettner · 2020-04-27T23:17:31Z

@rejas the unit page is failing because it needs a backend to build the IIFE wrapped versions of the modules. looking into the integration page now

patrickkettner · 2020-04-28T02:20:29Z

beh. bad news. a number of detects are failing.

take this

import Modernizr from '../src/Modernizr.js'
import featureDetectA from '../featureDetectA.js'

Modernizr.addTest('foo', a === 123)

export Modernizr.foo

The thing being exported by every feature-detect will be undefined, because addTest is just pushing onto an an array, and won't be triggered until testRunner() is called. So when one test relies on another detect, the originally one will be falsy at the time the second runs. This is also the case for people who import individual detects currently ,rather than importing a full build.

the modernizr-esm repo worked around this by calling testRunner() on every call to addTest(), but this ends up destroying performance for larger builds.

I am thinking we will need to wrap the export in some kind of function. When it is invoked, we test if it is being imported and if it is, execute the test and return the value. or something. IDK. Need to figure this one out

niksy · 2020-04-28T05:42:12Z

the modernizr-esm repo worked around this by calling testRunner() on every call to addTest(), but this ends up destroying performance for larger builds.

As I mentioned previously, maybe testRunner could keep cache of already run tests and if it hasn’t been run, run it, otherwise just go to the next test. Does that make sense?

patrickkettner · 2020-04-28T05:44:22Z

@niksy testRunner should only be running each test once anyway. As of now, testRunner hasnt been called, so there is nothing to be cached. having it called while other tests are being instrumented can impact their outcome, so it was trying to be avoided

niksy · 2020-04-28T05:59:21Z

having it called while other tests are being instrumented can impact their outcome, so it was trying to be avoided

Yeah, I was afraid of that :)

Maybe every test should be callable function, and that way we can keep consistency with async tests with test listeners?
Since we already push for v4, might as well introduce different and cleaner concept of test running.

Just throwing ideas :)

Improved language, clarity, etc. hashtagContinuousImprovement

patrickkettner · 2020-05-04T02:17:19Z

I haven't had a ton of time to fix this yet, but the first thought has been to modify the rollup function to insert a testRunner() call before any imported feature-detect. This can be modified for the prepublish script so that people can

import f from 'Modernizr/fakeFeatureDetectName.js

the idea bring to reduce the number of times testRunner is called to the absolute minimum, without changing the interaction that people expect in modernizr

WDYT @niksy @rejas

niksy · 2020-05-04T08:03:21Z

@patrickkettner yeah, that seems valid! Maybe insert it just before first export declaration?

rejas · 2020-04-24T19:42:03Z

README.md

@@ -74,13 +74,6 @@ To see all available options run:
 ./bin/modernizr
 ```

-Or to generate everything in 'config-all.json' run this with npm:


obviously has to be updated :-)

rejas · 2020-04-24T19:42:48Z

bin/modernizr

@@ -39,30 +37,23 @@ var yargs = require('yargs')
    alias: 'uglify',
    describe: 'uglify/minify the output'
  })
-  .options('s', {


this option is gone for good?

rejas · 2020-04-24T19:44:37Z

bin/modernizr

@@ -133,16 +124,16 @@ if (argv.o || argv.f) {
        // special case around it :[
        if (prop === 'setClasses') {
          return 'setClasses';
-        } else if (_.isUndefined(obj)) {
-          throw new Error('invalid key value name - ' + prop);
+        } else if (typeof obj === 'undefined') {


isnt (obj === undefined ) enough these days?

rejas · 2020-04-24T19:46:46Z

feature-detects/ambientlight.js

@@ -6,12 +6,17 @@
  "notes": [{
    "name": "W3C Spec",
    "href": "https://www.w3.org/TR/ambient-light/"
-  }]
+  }],
+  "caniuse": "ambient-light"


already in there

rejas · 2020-04-24T19:47:33Z

feature-detects/audio.js

@@ -4,6 +4,7 @@
  "property": "audio",
  "caniuse": "audio",
  "tags": ["html5", "audio", "media"],
+  "caniuse": "audio",


already in there

rejas · 2020-04-27T20:34:01Z

feature-detects/textencoding.js

+Modernizr.addTest('textdecoder', result.textdecoder);
+
+
+export let textencoder = Modernizr.textencoder


why is here an export let ? why is that special?

rejas · 2020-05-04T08:28:15Z

feature-detects/postmessage.js

@@ -13,15 +13,20 @@
 }
 !*/
 /* DOC
-Detects support for the `window.postMessage` protocol for cross-document messaging.
+Detects support for the `self.postMessage` protocol for cross-document messaging.


rejas · 2020-05-04T09:56:14Z

lib/config-all.json

    "mq",
    "prefixed",
    "prefixes",
    "prefixedCSS",
-    "setClasses",


why is that gone?

and could you document those changes (in the options) in the README too?

rejas · 2020-05-04T13:01:02Z

test/browser/src/testProps.js

      // Everyone supports fontSize
-      expect(testProps(['fontSize'])).to.be.equal(true);


you dont like the "be"s?

rejas · 2020-05-04T13:07:26Z

src/html5shiv.js

@@ -1,338 +1,339 @@
-define(['isSVG'], function(isSVG) {


that would be a thing: an update to html5shiv for es6 :-)

rejas · 2020-05-04T13:12:15Z

I haven't had a ton of time to fix this yet, but the first thought has been to modify the rollup function to insert a testRunner() call before any imported feature-detect. This can be modified for the prepublish script so that people can
import f from 'Modernizr/fakeFeatureDetectName.js
the idea bring to reduce the number of times testRunner is called to the absolute minimum, without changing the interaction that people expect in modernizr

WDYT @niksy @rejas

Sounds like a plan. Finished my review of the patch :-) Mostly documentation issues I'd say

KuraFire · 2020-07-02T00:34:47Z

What is the plan for handling the requested changes here? (In this herculean effort that predates the Trump administration, including the election itself… :D)

Markel · 2020-07-04T17:32:14Z

I've created a gist that adds the Avif that I introduced in #2539 so I can alleviate a little bit of the work
https://gist.github.com/Markel/d587ab198a9e1552c0cf0eddcff4f313. Take into account the polyfills and those kind of things.

Anything else I can work on just ping me 😄

niksy · 2020-08-21T07:36:35Z

@patrickkettner @rejas where are we with this? What can we do to continue with this effort?

rejas · 2020-08-21T12:44:20Z

hi @niksy yeah, somebody needs to pick up the PR if @patrickkettner doesnt respond or doesnt find the time. Unfortunalty I dont have the time these days for such work myself so any help is appreciated....

Markel · 2020-08-21T14:50:59Z

Maybe we merge the PR into a v4 branch so it is possible to work on it @rejas ?

rejas · 2020-08-21T15:53:08Z

Merging it in an unfinished state into the v4 branch I think is not good. Because what happens when the second maintainer also doesnt have time anymore? Then we have a stale branch that needs to be cleaned up again.

And sicne it has merge conflicts anyway, it needs to be forked by whoever wants to work on it anway before we can merge it into any official branch.

RiZKiT · 2020-11-23T13:44:09Z

Thanks a lot for your afford so far! But 450 changes files is not an easy branch, hope you get it merged, the longer it stays like this, the harder it will be.

If someone else stumbled upon this issue and read so far, there is also https://github.com/niksy/modernizr-esm to try out.

patrickkettner force-pushed the amd-conversion branch 2 times, most recently from ba6faeb to 132523a Compare October 17, 2016 22:24

SimenB mentioned this pull request Jan 24, 2017

Include built modernizr in npm package #1697

Closed

rejas mentioned this pull request May 22, 2018

State of the project? #2278

Closed

rejas assigned patrickkettner Jul 4, 2018

rejas added this to the Modernizr 4.0 milestone May 18, 2019

rejas mentioned this pull request Mar 17, 2020

rewrite in es6 modules #1333

Open

rejas removed this from the Modernizr v4.0 milestone Mar 17, 2020

patrickkettner force-pushed the amd-conversion branch from 132523a to 52eb255 Compare April 24, 2020 06:32

patrickkettner force-pushed the amd-conversion branch 3 times, most recently from f8932c4 to 5ec88fa Compare April 24, 2020 07:36

patrickkettner force-pushed the amd-conversion branch from 5ec88fa to a75f273 Compare April 24, 2020 07:39

patrickkettner force-pushed the amd-conversion branch from a75f273 to 67a08a0 Compare April 24, 2020 07:44

patrickkettner force-pushed the amd-conversion branch 2 times, most recently from 89ab3a4 to f91e1f1 Compare April 24, 2020 08:09

patrickkettner force-pushed the amd-conversion branch 2 times, most recently from 8751887 to 5705e85 Compare April 24, 2020 08:22

rejas self-requested a review April 24, 2020 08:27

patrickkettner added 5 commits April 27, 2020 14:55

ESM update - fixes for windows

20f4da8

update readme, remove deprecations

8a32164

fix bad metadata

5611127

remove dupliate test

6719c54

make sure everything is using contains

a8d0baa

patrickkettner force-pushed the amd-conversion branch from abb0244 to a8d0baa Compare April 27, 2020 18:56

patrickkettner referenced this pull request in Modernizr/modernizr-neue May 2, 2020

Updated documentation

8c97f17

Improved language, clarity, etc. hashtagContinuousImprovement

rejas requested changes May 4, 2020

View reviewed changes

Markel mentioned this pull request Aug 19, 2020

Flex gap detection does not work when inside display:none iframe #2590

Open

This was referenced Aug 30, 2020

Road to v4 🎉 #2594

Open

Add simple test for the property value #2595

Closed

Markel mentioned this pull request Dec 20, 2020

Can we re-run tests ? #1911

Open

niksy mentioned this pull request Nov 9, 2021

Module not found: Error: Cannot resolve module 'ModernizrProto' #2012

Closed

		Modernizr.addTest('textdecoder', result.textdecoder);


		export let textencoder = Modernizr.textencoder

		// Everyone supports fontSize
		expect(testProps(['fontSize'])).to.be.equal(true);

convert to es6 modules #2108

Are you sure you want to change the base?

convert to es6 modules #2108

Conversation

patrickkettner commented Oct 17, 2016 • edited

TODO BEFORE MERGE

rejas commented Jul 4, 2018

niksy commented Aug 8, 2018

niksy commented Jan 25, 2019 • edited

rejas commented Jan 30, 2019

niksy commented Jan 30, 2019

rejas commented Feb 19, 2019

patrickkettner commented Apr 24, 2020 • edited

niksy commented Apr 24, 2020

patrickkettner commented Apr 24, 2020

niksy commented Apr 24, 2020

patrickkettner commented Apr 24, 2020

rejas commented Apr 27, 2020

rejas commented Apr 27, 2020

patrickkettner commented Apr 27, 2020

patrickkettner commented Apr 28, 2020

niksy commented Apr 28, 2020

patrickkettner commented Apr 28, 2020

niksy commented Apr 28, 2020

patrickkettner commented May 4, 2020 • edited

niksy commented May 4, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rejas commented May 4, 2020

KuraFire commented Jul 2, 2020

Markel commented Jul 4, 2020 • edited

niksy commented Aug 21, 2020 • edited

rejas commented Aug 21, 2020

Markel commented Aug 21, 2020

rejas commented Aug 21, 2020

RiZKiT commented Nov 23, 2020

patrickkettner commented Oct 17, 2016 •

edited

niksy commented Jan 25, 2019 •

edited

patrickkettner commented Apr 24, 2020 •

edited

patrickkettner commented May 4, 2020 •

edited

niksy commented May 4, 2020 •

edited

Markel commented Jul 4, 2020 •

edited

niksy commented Aug 21, 2020 •

edited