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

Fix ove and ui imports for vitest #25

Merged
merged 4 commits into from
Sep 13, 2023
Merged

Fix ove and ui imports for vitest #25

merged 4 commits into from
Sep 13, 2023

Conversation

smeng9
Copy link
Contributor

@smeng9 smeng9 commented Sep 11, 2023

Hi @tnrich, we just noticed the https://github.com/TeselaGen/openVectorEditor gets moved to here.
We are trying to switch to the @teselagen/ove with our current build system and expecting to be simple name replacements but we found some issues with our testing.

As a follow up of #13 and #16 , We noticed the exports needs to be done correctly with the correct format in order to be used in vitest. I changed the file name format in vite.react.config.ts and updated all package.json to use the updated files. Some package.json not exporting the files causing tests to fail.

I have also updated your example to demonstrate vitest. We need to add vitest dependency, update vite.config.js configs, fix the test.jsx file extension, and update the test cases.

If you publish a new versions of ove and ui, and bump the internal ove's dependency on ui with this merge request, the test cases will probably pass. For now it throws various errors complaining package exports are not setup correctly.

@smeng9
Copy link
Contributor Author

smeng9 commented Sep 12, 2023

I also think you can put a deprecation on the https://www.npmjs.com/package/open-vector-editor and archive https://github.com/TeselaGen/openVectorEditor so we would immediately know next time when we perform dependency install.

Copy link
Collaborator

@tnrich tnrich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

@tnrich tnrich merged commit 533b5ab into TeselaGen:master Sep 13, 2023
@smeng9
Copy link
Contributor Author

smeng9 commented Sep 13, 2023

Hi @tnrich, I realized there are some additional issue and the tests are not passing.

@smeng9
Copy link
Contributor Author

smeng9 commented Sep 13, 2023

  1. The https://www.npmjs.com/package/@teselagen/ui seems is not updated. In the package.json file https://www.npmjs.com/package/@teselagen/ui?activeTab=code it is still not exporting the index.cjs.js and index.es.js files.

  2. The published ove package.json is referencing an old copy of ui of 0.3.13 here https://www.npmjs.com/package/@teselagen/ove?activeTab=code .

  3. Since we publish both es and cjs files, we have to omit the type field in package.json in the published packages. Then the bundler can determine the correct type, for now webpack failed to build.

@tnrich
Copy link
Collaborator

tnrich commented Sep 13, 2023

@smeng9 whoops, yeah looks like I only published ove, re-publishing both ui and ove now

@smeng9
Copy link
Contributor Author

smeng9 commented Sep 13, 2023

Also need to merge #26 to fix the file type used by web pack bundler

@smeng9
Copy link
Contributor Author

smeng9 commented Sep 13, 2023

All vitest passing in the example-demos/oveViteDemo now. Thanks!

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.

2 participants