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

bug: css source maps not inlining original content #24441

Closed
4 of 6 tasks
hilzfa opened this issue Dec 18, 2021 · 9 comments · Fixed by #24514
Closed
4 of 6 tasks

bug: css source maps not inlining original content #24441

hilzfa opened this issue Dec 18, 2021 · 9 comments · Fixed by #24514
Assignees
Labels
package: core @ionic/core package type: bug a confirmed bug report

Comments

@hilzfa
Copy link

hilzfa commented Dec 18, 2021

Prerequisites

Ionic Framework Version

  • v4.x
  • v5.x
  • v6.x

Current Behavior

I am not really sure, if it's a bug or more a missing peace which should be a nice-to-have.

I am using @ionic/react to develop my react app using the react-scripts in version 5.0.0. With v5 they introduced the source-map-loader by default which leads to an error message in building the ionic application because of missing source maps in @ionic/react.

Screenshot 2021-12-18 114140

These "errors" are classified as WARNING but it is cluttering the console as you can see in the screenshot above.
Is there a plan to add source maps to the @ionic/react package?

Currently, I can mitigate this behavior by disabling the source-map-loader at all. But this is not a nice solution. Maybe this would be adapted anyway when updating @ionic to support react-scripts@5

Expected Behavior

The expected behavior for me is, that there are source maps for @ionic/react and the webpack source-map-loader could load these. Currently, it's not a bug in the sense of something does not work. It's more a feeling that it would be nice to mitigate these warnings by adding the necessary source maps.

Please let me know, if there any plans to add them. I mean the react-scripts in version 5 are quite new and i was not expecting everything working without problems. Don't get me wrong. 😄

Steps to Reproduce

  • create a new react ionic v6 project with the ionic cli ionic start myApp blank --type=react

  • install the latest react-scripts version yarn add exact [email protected] or npm i [email protected]

  • start the application yarn start or npm start

  • look at the console to recognize the bunch of warnings regarding parsing error of source maps in @ionic/react/src/*

Code Reproduction URL

No response

Ionic Info

Ionic:

Ionic CLI : 6.13.1 (C:\Users\kingl\AppData\Roaming\npm\node_modules@ionic\cli)
Ionic Framework : @ionic/react 6.0.1

Utility:

cordova-res : not installed
native-run : 1.5.0

System:

NodeJS : v16.13.1 (C:\Program Files\nodejs\node.exe)
npm : 8.3.0
OS : Windows 10

Additional Information

No response

@ionitron-bot ionitron-bot bot added the holiday triage issues that were created during holiday period label Dec 18, 2021
@ionitron-bot
Copy link

ionitron-bot bot commented Dec 18, 2021

Thanks for the issue! This issue has been labeled as holiday triage. With the winter holidays quickly approaching, much of the Ionic Team will soon be taking time off. During this time, issue triaging and PR review will be delayed until the team begins to return. After this period, we will work to ensure that all new issues are properly triaged and that new PRs are reviewed.

In the meantime, please read our Winter Holiday Triage Guide for information on how to ensure that your issue is triaged correctly.

Thank you!

@babycourageous
Copy link
Contributor

Just wanted to comment that i experienced this too but only after upgrading to react-scripts v5. While running v4 that comes with Ionic 6 i didn't get any of those sourcemap errors.

I'm guessing that the react-scripts major upgrade probably has some breaking changes that still need incorporating into Ionic React v6. I'm currently sticking with react-scripts v4.x (as well as v5 of the react router) in my projects until those upgrades are officially incorporated,

Ionic Team -
here's a clean Tabs repro with an upgraded react-scripts package for any testing you'd like to do:
https://github.com/babycourageous/ionic-react-scripts-5-sourcemmaps

@hilzfa
Copy link
Author

hilzfa commented Dec 26, 2021

@babycourageous - Thanks for confirmation. I can confirm that there is no issue with react-scripts v4.x. That's the reason why I would assume that this is more a missing feature to support react-scripts v5.x than a bug in the current version. Just wanted to mention it. 😄

@liamdebeasi liamdebeasi self-assigned this Jan 5, 2022
@liamdebeasi
Copy link
Contributor

liamdebeasi commented Jan 5, 2022

Thanks for the issue. We do have source maps in Ionic, but it looks like source-map-loader is looking in the wrong place for the source maps. If you go into node_modules/@ionic/react/css you should see files such as core.css and core.css.map.

I will do some more digging and reply here when I have more to share.

@liamdebeasi
Copy link
Contributor

Actually, it looks like source-map-loader is loading core.css.map. The problem is that we also reference the following in core.css.map:

{"version":3,"sourceRoot":"","sources":["../src/css/core.scss","../src/themes/ionic.mixins.scss","../src/themes/ionic.globals.scss","../src/components/menu/menu.ios.vars.scss","../src/components/menu/menu.md.vars.scss"]

Since we do not ship the source files with built versions of Ionic, these referenced files do not exist and source-map-loader throws an error. What we should be doing is inlining the original code using the sourcesContent option when generating the source maps. This is what we do for the Ionic React JS code and that is working as intended.

@liamdebeasi liamdebeasi changed the title bug: Missing source maps in @ionic/react bug: css source maps not inlining original content Jan 5, 2022
@liamdebeasi liamdebeasi added package: core @ionic/core package type: bug a confirmed bug report and removed holiday triage issues that were created during holiday period labels Jan 5, 2022
@liamdebeasi
Copy link
Contributor

Thanks for the issue. This has been resolved via #24514, and a fix will be available in an upcoming release of Ionic Framework.

@babycourageous
Copy link
Contributor

Thanks @liamdebeasi !

@hilzfa
Copy link
Author

hilzfa commented Jan 11, 2022

Thanks @liamdebeasi for fixing this. All the best!

@ionitron-bot
Copy link

ionitron-bot bot commented Feb 10, 2022

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Ionic, please create a new issue and ensure the template is fully filled out.

@ionitron-bot ionitron-bot bot locked and limited conversation to collaborators Feb 10, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
package: core @ionic/core package type: bug a confirmed bug report
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants