TeselaGen / tg-oss

Teselagen Open Source modules
https://teselagen.github.io/tg-oss/
MIT License
40 stars 17 forks source link

Omit package type if includes both es and cjs #26

Closed smeng9 closed 1 year ago

smeng9 commented 1 year ago

Hi @tnrich

I have address the issue 3 here in the follow up comment https://github.com/TeselaGen/tg-oss/pull/25#issuecomment-1718141694 for webpack bundler

Additionally in order to address issue 1 and 2 to make the tests in oveViteDemo working, the @teselagen/ui package seems need to be republished

tnrich commented 1 year ago

@smeng9 I'm not sure I can take this fix since I think our code doesn't play well with ove/ui if the package type isn't common.. I'll need to do a bit of digging into this on our end.

smeng9 commented 1 year ago

If we use the type: commonjs while we have both es and cjs files, it misleads the web pack bundler. If we omit this field, web pack can figure the file type out by itself.

Vite seems does not have the issue.

smeng9 commented 1 year ago

That really depends on the bundler setting.

We can keep this open and you can play around to see if type: commonjs is necessary.

tnrich commented 1 year ago

@smeng9 looks like it isn't an issue for us to remove the .type field from the package.json. I've published new versions of ove/ui with it removed. Lemme know if that fixes it for you

smeng9 commented 1 year ago

The 0.3.17 works for oveViteDemo and vitest but has issue for oveWebpackDemo. I tested by manually set to that version in https://github.com/TeselaGen/tg-oss/blob/47da3e261a1df69e5475c4a0cf7b062dd6b73566/example-demos/oveWebpackDemo/package.json#L6

The 0.3.18 has issue for vitest in oveViteDemo, but it works both for webpack and vite.

I have checked the type field package.json. It is changed to module, but we have both es and cjs file format published, so the bundler gets confused again. I manually modified the file in node_modules folder and If we don't have the type field at all, all of vite, vitest, and webpack would pass.

tnrich commented 1 year ago

@smeng9 ok thanks for looking into this more. I'm re-publishing with the type field completely removed this time I believe.

smeng9 commented 1 year ago

Thank! All passing now.