pkg icon indicating copy to clipboard operation
pkg copied to clipboard

feat: ESM modules support

Open robertsLando opened this issue 2 years ago • 21 comments

Ref #1291

To run test: FLAVOR=test-00-esm npm test

robertsLando avatar Sep 30 '21 13:09 robertsLando

Sorry the test has fails and i forgot to delete it, when it works the vacoom automatically removes it.

Just to be clear tests will not pass as this is not a fix to #1291 but it just reproduce the problem, I tried to fix it without success. Do you have any clue what's not working?

robertsLando avatar Sep 30 '21 15:09 robertsLando

Sorry the test has fails and i forgot to delete it, when it works the vacoom automatically removes it.

Just to be clear tests will not pass as this is not a fix to #1291 but it just reproduce the problem, I tried to fix it without success. Do you have any clue what's not working?

#782

We have no support for ES6 modules yet. The walker can't parse the dependencies and include them in the payload.

jesec avatar Oct 01 '21 03:10 jesec

The strange thing is that the detector actually identifies import statements: https://github.com/vercel/pkg/blob/59125d1820cdb380f3f592604bcfd7c017495e77/lib/detector.ts#L258

Also in latest ncc updates now it outputs esm modules so it's not a solution at all, see this. I think we need to add a support to esm modules as with node 14+ them are supported. If you could point me on the changes needed I could try to fix this.

robertsLando avatar Oct 01 '21 06:10 robertsLando

@jesec news here? Are you sure the problem is in walker so?

robertsLando avatar Oct 06 '21 07:10 robertsLando

Ok I answer my self. The problem is that actually resolve package cannot handle esm modules yet: https://github.com/browserify/resolve/issues/222

A possible fix could be try using https://www.npmjs.com/package/enhanced-resolve

We could also consider usign https://github.com/bytenode/bytenode as fabricator

robertsLando avatar Oct 06 '21 08:10 robertsLando

This could be a starting point: https://github.com/vitejs/vite/pull/5593

robertsLando avatar Nov 10 '21 10:11 robertsLando

Our use case is considerably more complicated. enhanced-resolve is nowhere near to a drop-in replacement for resolve in our case.

We would have to study the APIs of the enhanced-resolve and rewrite our logics.

jesec avatar Nov 10 '21 11:11 jesec

We would have to study the APIs of the enhanced-resolve and rewrite our logics.

I agree, in my tests I was just tring to see what happened using that as a separete tool then the plan it's to completely replace resolve. I think this issue should be the top in our todos (giving the amount of related open bugs), do you want to do some tries? Test is already setted up

robertsLando avatar Nov 10 '21 12:11 robertsLando

I have been using es6 in my projects, but before I run pkg I transpile them to commonjs. This seems to work well. It would be great if this support landed natively or if you just build in the transpiling part so I didn't have to.

hcldan avatar Jan 21 '22 20:01 hcldan

I transpile them to commonjs

Do you use webpack for this?

robertsLando avatar Jan 24 '22 08:01 robertsLando

rollup worked really well, as I don't need the features of webpack, I just need my modules to be cjs in the end.

package.json

{
  "scripts": {
     "build": "npm-run-all build:*",
     "build:1": "rimraf dist",
     "build:2": "mkdirp dist",
     "build:3": "rollup index.js -o dist/built.js --format cjs",
     "build:4": "pkg dist/built.js --targets linux,win --out-path dist"
  }
}

hcldan avatar Jan 24 '22 15:01 hcldan

sourcemaps are a pain... so native support would be fantastic

hcldan avatar Jan 24 '22 15:01 hcldan

are there any updates or workarounds for this? I really need to distrabute my package ASAP

RUGMJ avatar Mar 03 '22 17:03 RUGMJ

are there any updates or workarounds for this? I really need to distrabute my package ASAP

You can use caxa, because they are supporting ESM.

kamack38 avatar Mar 03 '22 18:03 kamack38

I've commented above for a decent workaround that I'm using

rollup can be configured to transpile and bundle all es modules in a file and defer all commonjs modules to the default require.

  "scripts": {
    "test": "npm-run-all test:*",
    "test:1": "eslint",
    "test:2": "NODE_OPTIONS=--experimental-vm-modules npx jest",
    "start": "node .",
    "build": "npm-run-all build:*",
    "build:1": "rimraf dist",
    "build:2": "mkdirp dist",
    "build:4": "rollup --config rollup.config.js",
    "build:5": "pkg dist/nwsp.js --targets linux,win --out-path dist"
  },

rollup.config.js

import nodeResolve from 'rollup-plugin-node-resolve';

export default {
  input: 'index.js',
  output: {
    file: 'dist/nwsp.js',
    format: 'cjs',
  },
  plugins: [
    nodeResolve({
      modulesOnly: true,
    }),
  ],
};

There are tons of options, look them over. This gives you one big file with all es5 turned into commonjs. Only big drawbacks here are complexity (it would be nice if this all just worked) and sourcemap support. Rollup can output sourcemaps (even inline) but I cant get stacks reported by node to use that info.

hcldan avatar Mar 04 '22 16:03 hcldan

Any updates on this feature?

gaby avatar Apr 22 '22 12:04 gaby

@robertsLando Are there plans to merge this PR? Some dependencies like https://github.com/sindresorhus/got only work with ESM modules now and encourage switching (https://gist.github.com/sindresorhus/a39789f98801d908bbc7ff3ecc99d99c).

The options are to stay on older versions of dependencies or to stop using pkg but it's better if we don't have to do either.

rightaway avatar Jun 20 '22 06:06 rightaway

This PR doesn't fixes this, @jesec has a working implementation but not finished yet and I dunno if there is any ETA on it

robertsLando avatar Jun 20 '22 06:06 robertsLando

This pull-request is stale because it has been open 90 days with no activity. Remove the stale label or comment or this will be closed in 5 days. To ignore this pull-request entirely you can add the no-stale label

github-actions[bot] avatar Sep 19 '22 00:09 github-actions[bot]

Any reason why stale bot is trying to close PR's?

gaby avatar Sep 19 '22 00:09 gaby

Any reason why stale bot is trying to close PR's?

I'd say it's

because it has been open 90 days with no activity

CleyFaye avatar Sep 19 '22 11:09 CleyFaye

Any updates on it?

angrymouse avatar Sep 25 '22 21:09 angrymouse

Any news at all?

edosrecki avatar Nov 04 '22 17:11 edosrecki

I think there is another issue here: even if the files are bundled into the virtual filesystem, it cannot be executed. I'm guessing node's ESM resolver does not use the patched filesystem.

For example, you can bundle an ESM file alongside an CJS file, using the CJS file as entrypoint. Then run stuff in the CJS file that looks vaguely like this:

// entrypoint.cjs:
// Prove that the file exists in VFS
console.log(fs.readdirSync('/snapshot'));
console.log(fs.readFileSync('/snapshot/bin.mjs'));
// But import fails
import('/snapshot/bin.mjs');

Gives a cryptic error:

Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/snapshot/bin.mjs' imported from /snapshot/entrypoint.cjs
Did you mean to import /snapshot/bin.mjs?

Looking closely at that error, it says i cannot find the module, then it suggests exactly the same path. I bet the suggestion comes from different code which is using the VFS.

cspotcode avatar Jan 31 '23 23:01 cspotcode

Unfortunately I have no clue about how to actually implement such feature, I would need some support on this

robertsLando avatar Feb 01 '23 07:02 robertsLando

@robertsLando Perhaps you could create a custom loader that uses the virtual filesystem. Here are some example implementations from the node docs: https://nodejs.org/dist/latest-v19.x/docs/api/esm.html#loaders

Haringat avatar Apr 06 '23 00:04 Haringat

@Haringat Thanks for your suggestion, BTW I don't think that would solve the issue, right now the problem is we don't have a way to resolve the modules used with esm, enhanced resolve should solve this issue but I have no clue how to make it work

robertsLando avatar Apr 07 '23 13:04 robertsLando

@robertsLando But with the loader you can specify a custom resolve hook to do just that. It gets the URL it is supposed to resolve and the URL of the importing module.

Haringat avatar Apr 07 '23 15:04 Haringat

It would not work for dynamically loaded modules I think? I mean an async import for example will not be catched by it on startup

robertsLando avatar Apr 07 '23 16:04 robertsLando

@robertsLando I just tested it with an HTTP/HTTPS loader and it was also invoked for a dynamic import.

However, I have absolutely no clue how createRequire would work in such a case. That would be an interesting test, though.

Edit: I tried it with that HTTP loader and a js file on localhost, but node.js wisely blocks imports from node builtin modules with the error "import of 'node:module' by http://127.0.0.1:8080/index.mjs is not supported: only relative and absolute specifiers are supported." I guess that would not be a problem in your case, though, as you could use the file protocol for your vfs and have node not even realize what is going on.

Haringat avatar Apr 07 '23 19:04 Haringat