#6447 closed change (rejected)

Switch to Harmony modules

Reported by: mjethani Assignee:
Priority: Unknown Milestone:
Module: Core Keywords:
Cc: kzar, sergz, sebastian, agiammarchi, hfiguiere, greiner, fhd Blocked By: #6448
Blocking: #6425 Platform: Unknown / Cross platform
Ready: no Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

https://codereview.adblockplus.org/29715555/

Description

Background

ES6 offers a native alternative to CommonJS modules that is mostly compatible, with a cleaner syntax.

e.g. CommonJS

const {foo} = require("foo");

function bar()
{
  foo();
}

exports.bar = bar;

Harmony

import {foo} from "foo";

export function bar()
{
  foo();
}

What to change

Update CommonJS-style modules to use new ES6 syntax instead.

Update ESLint config to parse JS files as modules rather than as scripts.

Change History (8)

comment:1 Changed 21 months ago by mjethani

  • Review URL(s) modified (diff)

comment:2 Changed 21 months ago by mjethani

It seems V8 still doesn't support ES6 modules by default, but the feature is available behind a --experimental-modules flag.

comment:3 Changed 21 months ago by sergz

  • Blocked By 6448 added

I have created an issue to look into it in libadblockplus.

comment:4 follow-up: Changed 21 months ago by agiammarchi

as previously mentioned, I'm part of the node/modules Working Group [1]

it's a very bad time for working on anything ESM related because nobody knows how/when/what will be available.

there are also limitations in ESM that have no equivalent in CJS so if I can give you my 2 cents, I'd put this on hold for *at least* another month.

[1] https://github.com/nodejs/modules#modules-team

comment:5 in reply to: ↑ 4 ; follow-up: Changed 18 months ago by mjethani

Replying to agiammarchi:

it's a very bad time for working on anything ESM related because nobody knows how/when/what will be available.

Any updates on this? If not, we should probably close this issue for now and revisit it later.

comment:6 in reply to: ↑ 5 Changed 18 months ago by agiammarchi

Replying to mjethani:

Any updates on this?

Nothing too relevant, except even Node core dev/manager at JSConfEU stated it's not a good idea to use modules now.

I've also asked to be moved from member to observer, since there's too much going on there and I don't have time for that (and I feel well represented by people actually paid to follow all that).

As side note, it looks like developers in general are confident using @std/esm for the time being, and migrate later on, while Webpack users never cared much about anything, thinking Webpack will figure things out at some point and solve things for them.

I think the key, if we decide to implement ES modules, is to not abuse ESM. That simply means that relative / absolute paths should be fully resolvable (i.e.import $ from "./lib" is bad, while import $ from "./lib.js" is ok).

That also usually makes bundlers happier, but we need to double test that required modules installed via npm resolve properly.

Last, but not least, if we have conditional requires, switching to dynamic import(...) must be addressed too but top level await is not a 100% safe bet, since it's only at stage 2, but node modules might end up with that too. Meanwhile we might use import(...).then(...) when/if needed.

As summary: I'm not sure if I've helped at all making any decision, but maybe we should not rush this change as things are moving, and changing, fast (also for the module mapped/path resolution, addressed in here)

Last edited 18 months ago by agiammarchi (previous) (diff)

comment:7 Changed 18 months ago by mjethani

Sounds good. I'll close this for now.

comment:8 Changed 18 months ago by mjethani

  • Resolution set to rejected
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.