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

Specify module for Webpack 4 #11037

Merged
merged 1 commit into from
Jan 10, 2023
Merged

Specify module for Webpack 4 #11037

merged 1 commit into from
Jan 10, 2023

Conversation

ankane
Copy link
Contributor

@ankane ankane commented Jan 6, 2023

Hi, thanks for this project!

Webpack 4 currently loads ./dist/chart.cjs since it doesn't support exports: webpack/webpack#9509

@ankane
Copy link
Contributor Author

ankane commented Jan 6, 2023

Looks like it was broken by #11033 (which switched main)

Copy link
Collaborator

@dangreen dangreen left a comment

Choose a reason for hiding this comment

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

@ankane Hi. Thank you for the PR.

main field contains bundle suitable for nodejs

i'm ok with these changes, but this problem is relevant only for webpack <= 4 and can be solved by using this plugin

@etimberg
Copy link
Member

This seems ok, but I am a bit wary of all these changes in the package.json file. It seems like the bundler ecosystem is so fragmented that finding a solution that works for everyone is impossible.

@kurkle
Copy link
Member

kurkle commented Jan 10, 2023

This seems ok, but I am a bit wary of all these changes in the package.json file. It seems like the bundler ecosystem is so fragmented that finding a solution that works for everyone is impossible.

I think one mistake was to change anything else than adding the exports. Old stuff does not read the exports at all, and we had things working quite well.

@etimberg etimberg merged commit b2b881b into chartjs:master Jan 10, 2023
@ankane
Copy link
Contributor Author

ankane commented Jan 11, 2023

Thanks everyone!

@dangreen
Copy link
Collaborator

@etimberg @kurkle @LeeLenaleee Hi. How about to make bugfix release with it?

@kurkle
Copy link
Member

kurkle commented Jan 16, 2023

I think @LeeLenaleee suggested in slack, that it should be 4.2 because package.json changed. But I'd be fine releasing this kind of change as a bug fix.

@stockiNail
Copy link
Contributor

@kurkle @LeeLenaleee @dangreen @etimberg this PR should also solve another issue opened in annotation plugin (chartjs/chartjs-plugin-annotation#831). Maybe this PR could be marked as "chore" in order to have an evidence in the release note. I'm just thinking loud.

@stockiNail
Copy link
Contributor

Thank you @kurkle

@Sokhongg
Copy link

image

@Sokhongg
Copy link

can anyone one solve this problem I spend almost 4hours to fix this and still no clue thanks.

@dangreen
Copy link
Collaborator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants