acornjs / acorn-jsx

Alternative, faster React.js JSX parser
MIT License
651 stars 74 forks source link

Add support for ES Module #113

Open SaekiRaku opened 4 years ago

SaekiRaku commented 4 years ago

I've checked out the source code of acorn, so that decided to use the same bundler(rollup) to generate the ES Module version of acorn-jsx. And use index.mjs as the filename of the ES Module, just like acorn-jsx do.

However, I also noticed that acorn-jsx is zero-dependencies for now. I don't know if it's acceptable to include a new dependency. Actually, the easiest way to do that is RegExp + String replace, but without lexical analysis, it will bring new issues in the future.

At last, if someone wants to use acorn-jsx in Deno, you should configure an imports map like below:

{
    "imports": {
        "acorn": "https://cdn.jsdelivr.net/npm/acorn@7.2.0/dist/acorn.mjs"
    }
}

Because acorn-jsx has a property tokTypes that relies on acorn, in the ES Module of acorn-jsx it is imported by import "acorn";.

RReverser commented 4 years ago

Thanks for the PR, this is a great start!

However, I also noticed that acorn-jsx is zero-dependencies for now. I don't know if it's acceptable to include a new dependency.

These are devDependencies, and so perfectly fine since they will be installed only by maintainers.

I think what we might want here instead though is to make ES module the primary source, and auto-generate CommonJS instead.

This would make the code that maintainers actually work with use modern syntax, and CommonJS a production artifact for older versions of Node, which also makes it easier to deprecate it altogether in some far future when everyone migrates.

SaekiRaku commented 4 years ago

I've transformed all codes to ES6 import/export syntax, including the test. And use these to generate a CommonJS module.

But there is a problem with tokTypes, if use export { tokTypes } in the ES Module. Rollup will bundle up like below:

exports.default = acronJsx;
export.tokTypes = tokTypes;

That will change the require syntax for the CommonJS users. So, for now, I'm keeping the Object.defineProperty(acornJsx, "tokTypes", {...}) and commented export { tokTypes }.

RReverser commented 4 years ago

But there is a problem with tokTypes, if use export { tokTypes } in the ES Module. Rollup will bundle up like below:

Yeah that makes sense. Note that you couldn't use an export { tokTypes } because it's a getter anyway.

However, that property was added mainly for backwards compatibility and the right way to use it is now via the value returned from the factory function. I wonder if, together with this change, we could just get rid of it and make a major bump...

SaekiRaku commented 4 years ago

Okey, all left over comments were removed, and acornJsx is changed to function declaration.

But I'm not very familiar with acorn, I just start to using it recently. 😅I'm not quite understanding how tokTypes works.

Alaboudi1 commented 4 years ago

Any update on this?

elevatebart commented 4 years ago

Thank you both for the hard work!!! How can I help bring it closer to ready to merge?

SaekiRaku commented 4 years ago

@elevatebart Emm, in this PR, I also changed the test scripts to use import Syntax. So they need esm as devDependencies to run the test scripts. But the travis-ci config didn't install all dependencies, therefore the ci checks have failed.

thescientist13 commented 9 months ago

Any updates available or anything anyone can do to help push this along?