Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature request: Add support for esm. #52

Closed
coreyfarrell opened this issue Jun 20, 2018 · 11 comments
Closed

Feature request: Add support for esm. #52

coreyfarrell opened this issue Jun 20, 2018 · 11 comments

Comments

@coreyfarrell
Copy link
Member

I'd like to request that esm be added if possible. Bad news I just tried adding it and couldn't get it to work. I added '.esm.mjs': 'esm', to extensions and '.esm.mjs' to jsVariantExtensions. This caused the following from gulp:

$ gulp
[21:05:15] Requiring external module esm
/home/coreyfarrell/test/gulpfile.esm.mjs:1
(function (exports, require, module, __filename, __dirname) { import log from 'gulplog';
                                                                     ^^^

SyntaxError: Unexpected identifier
    at new Script (vm.js:74:7)
    at createScript (vm.js:246:10)
    at Object.runInThisContext (vm.js:298:10)
    at Module._compile (internal/modules/cjs/loader.js:670:28)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:713:10)
    at Module.load (internal/modules/cjs/loader.js:612:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:551:12)
    at Function.Module._load (internal/modules/cjs/loader.js:543:3)
    at Module.require (internal/modules/cjs/loader.js:650:17)
    at require (internal/modules/cjs/helpers.js:20:18)

The result was the same for gulp and npx gulp (global gulp-cli vs local gulp-cli).

@phated
Copy link
Member

phated commented Jun 20, 2018

Thanks for opening the issue! I think the implementation will be closer to Babel (with the custom function wrapper) but haven't looked into it yet.

@phated
Copy link
Member

phated commented Jun 29, 2018

@coreyfarrell I'm digging into this more but it seems like it'd require changes on the esm side because they expect the consumer to use a returned require-style function but we expect things to be registered onto a extension.

@phated
Copy link
Member

phated commented Jun 29, 2018

Actually, I think I found the answer at standard-things/esm#433

@phated phated closed this as completed in 7205800 Jun 29, 2018
@phated
Copy link
Member

phated commented Jun 29, 2018

Need to do some testing on the changes using rechoir before I publish.

@coreyfarrell
Copy link
Member Author

I just did some local testing against gulp, one issue I ran into it doesn't seem to respect any of the ways to set ESM options. To enable debug I had to edit interpret to use var esmLoader = hook(module, {debug: true});.

Note I'm looking into debug to find out why import {rollup} from 'rollup'; doesn't work. I think it's an esm bug, I'm able to reproduce this issue with a regular node -r esm script.mjs.

@phated
Copy link
Member

phated commented Jun 29, 2018

@coreyfarrell I'm guessing they don't look up options when it's used programmatically - might want to check that too.

@coreyfarrell
Copy link
Member Author

I got an explanation of the issue I was having with rollup. I had to drop my plan to use a dual mode main file in my own package, instead using "main": "index.js", "module": "index.mjs".

At this point I'm not sure how important it is to be able to set the ESM options, I'm able to use gulpfile.esm.js with the default options.

@coreyfarrell
Copy link
Member Author

@phated Would you be willing to publish an updated version that includes .esm.js? I've been using this for a while and once I understood the limitations of the esm module it works great for me. Even if I'm pointing to a @next version I'd prefer to use an npm based dependency over gulpjs/interpret#master.

@phated
Copy link
Member

phated commented Nov 3, 2018

Yeah, I can probably get something published this week.

@phated
Copy link
Member

phated commented Dec 27, 2018

@coreyfarrell potentially great news, I'm just wrapping up a complete test suite for this project and should have the ESM update published as soon as my Window's CI succeeds.

@phated
Copy link
Member

phated commented Dec 28, 2018

@coreyfarrell just published as 1.2.0!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants