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

Use commonjs module #26

Closed
wants to merge 1 commit into from
Closed

Use commonjs module #26

wants to merge 1 commit into from

Conversation

budziam
Copy link
Contributor

@budziam budziam commented Oct 16, 2019

This PR solves issue with using vuex-class-modules along with jest testing framework. Unfortunately Jest does not play well with ES6 module syntax. You can read more here: jestjs/jest#2550 (comment)

I think it's common approach to use commonjs module instead of es6 one.

@bodograumann
Copy link
Collaborator

The way I read that jest comment, you can “simply” transpile on your side.
I’m not convinced about switching to commonjs2, because we would loose tree-shaking. This might currently not be a big problem, but if the library grows in the future it will be.
At least we should provide a commonjs2 as well as an esm build.

@budziam
Copy link
Contributor Author

budziam commented Oct 23, 2019

So how about providing two builds? Can you do it?

@bodograumann
Copy link
Collaborator

Should be possible, in principle. My first instinct would be to add a rollup build-step. Not sure if I can find the time right now to flesh it out properly though.

@bodograumann
Copy link
Collaborator

Could you please check if this works for you: https://github.com/gertqin/vuex-class-modules/tree/feature/commonjs ?
I think you need to require("vuex-class-modules/dist").

@budziam
Copy link
Contributor Author

budziam commented Oct 26, 2019

There is one problem left. Typings are saved in lib directory. As a result, when using vuex-class-modules/dist, typescript does not see proper typings

Directory structure looks as follow:
image

@bodograumann
Copy link
Collaborator

That is true. I have switched to rollup-plugin-typescript2 because it generates the declaration files automatically.
In order to have no duplicate files, I moved the commonjs variant to lib/index.common.js. There was no lib/index.common.d.ts generated however. Not sure if that is a problem.
Please try it out. If the declarations are not found, could you also try to copy lib/index.d.ts to lib/index.common.d.ts please?

@gertqin
Copy link
Owner

gertqin commented Dec 23, 2020

Closing this in favor of #46

@gertqin gertqin closed this 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