Accessing module.id breaks

[tunnckoCore]

Log from the UI https://zeit.co/olstenlarck/tunnckocore-release-now-v2/agkrauawz

Error while initializing entrypoint: TypeError: u.startsWith is not a function
    at Object.<anonymous> (/var/task/user/index.js:152:281)
    at Object.<anonymous> (/var/task/user/index.js:153:30)
    at __webpack_require__ (/var/task/user/index.js:21:30)
    at Object.<anonymous> (/var/task/user/index.js:130:19)
    at Object.<anonymous> (/var/task/user/index.js:146:30)
    at __webpack_require__ (/var/task/user/index.js:21:30)
    at module.exports.module.exports.module.deprecate (/var/task/user/index.js:85:18)
    at Object.<anonymous> (/var/task/user/index.js:88:10)
    at Module._compile (module.js:652:30)
    at Object.Module._extensions..js (module.js:663:10)

[tunnckoCore]

It's probably even just after https://github.com/zeit/now-builders/commit/1443e5435bbb7b1cd0b05ba730f069c946f62857

[rauchg]

@tunnckoCore what package specifically causes this? Do you have a minimal reproduction? Can you re-open if this still happens, since it was from a few versions back?

Thank you!

[tunnckoCore]

Can think for a repro, but probably the most basic thing, starting from the example and definining @now/node@0.1. But anyway, I'll try it tomorrow if it is still happening. Thanks.

[tunnckoCore]

It seems it still fails. It's basically just a Probot app that I'm trying to deploy.
the status of the now github deployment check is green, but it seems to have error logs and does not opens the probot default app index page which usually is at /probot.

[rauchg]

What dependency is the one that ends up causing the issue @tunnckoCore ?

[tunnckoCore]

I'm not sure what you mean. But I'm using @now/node builder. The only sure thing is that such errors are not from my app, because I don't have any webpack stuff.

[rauchg]

CCing @igorklopov who can help debug

[igorklopov]

tunnckocore-release-now-v2 uses esm npm package. esm assumes that module.id is a string https://github.com/standard-things/esm/blob/880e03105f20d767d49b620717a2c78888ff2aa7/esm.js#L29, particularly calls it's startsWith method. In pure node.js env it is true - module.id in a string indeed. But when webpacked, that id is a number. You can test it by nccing console.log(typeof module.id, module.id);. A solution can be replacing module.i with String(module.i) in https://github.com/webpack/webpack/blob/e3e8a68dabc102d7f67fd4c5584baf293b21cddf/buildin/module.js#L16.

[tunnckoCore]

Hm. Sounds like it should be fixed there, but a temporary workaround for me would be to remove the esm.

Btw, the problem more of is that ncc is force-using the main instead of module. I definitely am not sure why this should be that way. But if it works as normal then the webpack will just resolve the src/index.js (which is the module field) instead of the esm "bridge" that i create (the main field). And I follow that pattern for all my repos and publishing packages.

A solution can be replacing module.i with String(module.i) in

Or in esm.

[igorklopov]

@tunnckoCore afaiu you use index.js explicitly in your now.json file. So ncc uses it as entrypoint for compiling your project. Can you please try specifying src/index.js and see what happens.

[tunnckoCore]

Hm. Make sense, yea. I'll try.

One more thought, can we define src: "." instead and so, it will search main and module fields from the package.json?

[tunnckoCore]

Strangely, src/index.js not works. It even does not build and no logs.

my fault, it probably will work.

[tunnckoCore]

OKey, src/index.js works. But still, can someone explain to me why we forcing to use main?

[igorklopov]

What makes you think we use entries of package.json? @now/node indeed uses package.json to install dependencies. But it does not use neither main, nor module, nor bin. You specify javascript file as src bypassing package.json and it's main entry. Correct me if i am wrong.

[igorklopov]

But seems you are right https://github.com/zeit/ncc/blob/ab31c82196e3157f3538aaebaf8809310b3ac794/src/index.js#L67

[tunnckoCore]

Yes, it forces Webpack to use main instead it's default resolution module, main.

but that's not really what node.js supports, so we reset it

It not really matter what node.js supports? You use Webpack to resolve everything, and it understands both CJS and ESM. And I think Webpack is making good that it defaults to module first and then main. Because not all have a module field. But those who have module field it will resolve it instead.

[rauchg]

@tunnckoCore that's the intent of this project. It's for Node.js codebases. Node doesn't support module. I'm re-opening as we definitely need to fix the module.id thing, which Node does support :)

[tunnckoCore]

Node doesn't support module.

You meant ESM? It partially supports and will support in future. Also it worth nothing to make ncc "just work" for both cjs and esm, jut by not forcing the resolving order.

Also, most of the projects already uses ESM, even without bundling, just with esm loader or the node's native flag (which is basically the esm loader under the hood). Otherwise, people who want to use ncc need to have two-step build: first run their build (which in most cases will be Webpack too) to make it cjs, then run ncc.

[rauchg]

The goal is that ncc just works for the cases where regular Node.js works. Even a slight change in semantics like what you are proposing will introduce surprising behavior.

ncc should introduce zero surprises when going from node program.js to ncc run program.js.

Re: extra transpilation steps. Yes, that'll be cumbersome. In that case you might want to go with Webpack directly, or go with TypeScript, which we intend to support (#31), if that covers your use-cases.

[rauchg]

And yes, I'm also excited about Node.js supporting ES modules! And ncc will mirror that support.

[tunnckoCore]

ncc should introduce zero surprises when going from node program.js to ncc run program.js.

Got it.

Basically, I don't need and don't want more than Nodejs v8 or v10+ and writing everything in ES Modules so it can be future proof from now, even if I die accidentally :D (bad black humor joke? ;d) I don't want build steps that's why I use esm loader to make it work for the cjs users. But for last 2 years I was always thinking to create similar thing to ncc, based on Rollup (as this started).

[guybedford]

Yeah we very specifically never want to be building "esm".

I would suggest we even mark this one as always being replaced with a "blank module".

[tunnckoCore]

Right.

Btw, we have a fix for this in standard-things/esm#681 already. Next one is Webpack. :)

[tunnckoCore]

I'm opening a new issue about the module field, because it's not directly related to this issue.

#236