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

🔧 Transpile to commonjs modules in addition to es2015 #46

Merged
merged 2 commits into from
Dec 23, 2020

Conversation

ftes
Copy link
Contributor

@ftes ftes commented Nov 26, 2020

  • Allow usage in backend app without a webpack/babel setup

@bodograumann
Copy link
Collaborator

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
Copy link
Contributor Author

ftes commented Nov 26, 2020

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 1cd45ba

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
Copy link
Collaborator

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
Copy link
Contributor Author

ftes commented Dec 15, 2020

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"
    }
  },

- Allow usage in backend app without a webpack/babel setup
@ftes ftes force-pushed the feature/transpile-to-commonjs branch from 16bc609 to 230ba18 Compare December 15, 2020 16:06
@ftes
Copy link
Contributor Author

ftes commented Dec 15, 2020

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).
33e3257

@gertqin gertqin mentioned this pull request Dec 23, 2020
@gertqin
Copy link
Owner

gertqin commented Dec 23, 2020

Looks good! Thanks for the contribution. :)

@gertqin gertqin merged commit 5de05e4 into gertqin:master Dec 23, 2020
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

Successfully merging this pull request may close these issues.

3 participants