search

mattpocock / xstate-codegen

A codegen tool for 100% TS type-safety in XState
MIT License
245 stars 12 forks source link

xstate-compiled Unexpected token 'export' #6

Closed redappleorigin closed 4 years ago

redappleorigin commented 4 years ago

I am using the library https://www.npmjs.com/package/ky and at the bottom of the index.js file it does an "export default createInstance()". The xstate-compiled package does not like that export line. Maybe an issue with es6 in node_modules?

mattpocock commented 4 years ago

Yes, this might be a duplicate of #3.

Could you send a reproduction? Are you targeting specific files with xstate-codegen, or running it on all files in your repo?

redappleorigin commented 4 years ago

Sorry for the wait, here is a repo that is just a copy of the xstate-compiled package with the fetchMachine modified to use ky. After installing and running, you will see the SyntaxError: Unexpected token 'export'.

https://github.com/redappleorigin/xstate-compiled-ky-issue

You should be able to do 'npm install' followed by 'npm start' to run the machine that is causing me problems.

I am working on a much large project that I am unable to share and ky is pretty much the only thing giving me trouble at the moment.

I looked at the other issue, but switching the export in ky from export default to export const does not solve the problem. Switching to module.exports does, but it's a package, so I am not really able to mess with it.

mattpocock commented 4 years ago

I'll likely have time to look at this on Sunday. Thanks for the detailed repro

mattpocock commented 4 years ago

@Andarist could this be an issue with the rollup configuration we're using? I assume that rollup is not compiling node_modules

Andarist commented 4 years ago

Yes, rollup is not compiling node_modules in our case. We offload loading node_modules to node as this sounds like a really sensible approach, especially given that node can cache those loaded modules effectively.

The problem is with the ky package that is publishing untranspiled ESM files and support for ESM stays experimental in node, so I don't really feel that supporting this right now is our top priority - I'm although saddened that you run into such problems as an early adopter. Could you maybe switch to using ky-universal or ky/umd?

To give some more technical details about this - we transpile loaded files to the CJS format and CJS can never require/import ESM in node (only ESM can import ESM). This is sensible as older versions of node don't even have an experimental ESM support enabled so we wouldn't be able to load files there at all.

More than that - ky is not even ready to be consumed by the current node's experimental support for ESM because it only has a .js file without specifying "type": "module" in its package.json, so the mode of interpretation is always set for it as script and not module. The only way to make things compatible with node for them would be to either use .mjs extension (as those are always loaded as modules) or put the mentioned "type": "module" in their package.json.

I strongly believe that trying to enable this right now on our side would just lead to myriad of other problems as the node's ecosystem is not yet ready for ESM and the interop story is incompatible with what people got used to when writing code for bundlers etc. For instance import { whatever } from 'lib' might work in your bundled application, but might now work in node if the lib is written using CJS format.

redappleorigin commented 4 years ago

Okay, fair enough. Thanks for looking into it. I will try ky-universal or ky/umd instead. Thanks again!