search

gertqin / vuex-class-modules

Typescript class decorators for vuex modules
MIT License
194 stars 20 forks source link

🔧 Transpile to commonjs modules in addition to es2015 #46

Closed ftes closed 3 years ago

ftes commented 3 years ago
bodograumann commented 3 years ago

That wrench confused me for a second. I though it was some new github feature :-D

There was already some effort in #26 regarding commonjs, but it did not receive any more feedback. (See also: https://github.com/gertqin/vuex-class-modules/tree/feature/commonjs) If the solution is really as simple as this, then I would be fine with it.

ftes commented 3 years ago

I just love my emojis 😉 Yes, I do think it is as simple as this. I tested my change locally. I'm happy to provide an example if you like.

Regarding #26

My solution can solve #26. But only if you specifically import from "vuex-class-modules/lib/commonjs" (because, of course, the default import is the es2015 module in package.json:"main": "lib/index.js"`).

Regarding existing branch feature/commonjs 1cd45bab

It looks like rollup was added. That is not necessary imho. We don't need to bundle the output, just expose it using commonjs module system. And tsc can do that out of the box.

bodograumann commented 3 years ago

Maybe we can reference both the esm build as well as the commonjs build in package.json. I'm never really sure what to put where. Perhaps set commonjs in main and esm in module?

Also it would be great if you could add a small description in the readme. I know it is getting longer and longer all the time, but it is the main documentation of this library :-)

ftes commented 3 years ago

Perhaps set commonjs in main and esm in module?

I agree. I tried to read up on this quickly and would suggest following this approach (conditional exports are no longer hidden behind a node feature flag): https://2ality.com/2019/10/hybrid-npm-packages.html#option-1-(experimental%2C-needs-conditional-exports)%3A-esm-and-commonjs-are-both-bare-imports

  "type": "module",
  "main": "commonjs/index.js",
  "module": "lib/index.js",
  "exports": {
    ".": {
      "require":  "./commonjs/index.js",
      "default": "./lib/index.js"
    }
  },
ftes commented 3 years ago

I have update the PR (as mentioned in my previous comment).

I have added a brief description to the readme.

Here is a working example for both the esm build (used by webpack) as well as the commonjs build (used by vanilla node v14). https://github.com/gertqin/vuex-class-modules/commit/33e325750a8801504e058d4e895207709aa190c6

gertqin commented 3 years ago

Looks good! Thanks for the contribution. :)