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(vite-plugin-angular): use babel to make transform results compati… #231

Merged
merged 1 commit into from
Jan 30, 2023

Conversation

goetzrobin
Copy link
Member

…ble with Webkit

Angular's compilation results use static blocks by default. These static blocks are not supported by Safari or other webkit based browsers. https://caniuse.com/mdn-javascript_classes_static_initialization_blocks We need to use babel transformations to make the output compatible with iOS. This is for serving content in development as well as building for production.

Closes #202

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

Which package are you modifying?

  • vite-angular-plugin
  • astro-angular
  • create-analog
  • router
  • platform
  • content

What is the current behavior?

Analog output is broken in Webkit based (Safari & all iOS) browsers.

Issue Number: #202

What is the new behavior?

Analog works as expected in Webkit based browsers

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

This is just a PR to share my findings and the patch that got it to work.
I hope someone with more insight can explain why it works and if there is a better way to achieve the same results.

@netlify
Copy link

netlify bot commented Jan 23, 2023

Deploy Preview for analog-app ready!

Name Link
🔨 Latest commit 568c153
🔍 Latest deploy log https://app.netlify.com/sites/analog-app/deploys/63d8166eba0caa00090b5902
😎 Deploy Preview https://deploy-preview-231--analog-app.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented Jan 23, 2023

Deploy Preview for analog-docs ready!

Name Link
🔨 Latest commit 568c153
🔍 Latest deploy log https://app.netlify.com/sites/analog-docs/deploys/63d8166e851480000b3354c6
😎 Deploy Preview https://deploy-preview-231--analog-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@brandonroberts
Copy link
Member

The second transform is only applied at build time to Angular libraries that need it. What's the error with Prism?

@goetzrobin
Copy link
Member Author

I'll need to find some time to look at it again, but it was something along the lines of:
Cannot find module ... index when index.js is expected in an mjs file.

I think applying the transformation only on build would be fine too. I think devs can live with ng serve only working in non WebKit browsers as long as the assets built work in Safari and every iOS browser

@netlify
Copy link

netlify bot commented Jan 30, 2023

Deploy Preview for analog-blog ready!

Name Link
🔨 Latest commit 568c153
🔍 Latest deploy log https://app.netlify.com/sites/analog-blog/deploys/63d8166eae6d89000940c14d
😎 Deploy Preview https://deploy-preview-231--analog-blog.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@goetzrobin
Copy link
Member Author

@brandonroberts the error was:
"default" is not exported by "node_modules/front-matter/index.js", imported by "node_modules/@analogjs/content/fesm2015/analogjs-content.mjs".

I updated the PR to what I think is the least intrusive way of getting
your production analog app working in safari by default,
while still allowing you to customize the behavior if needed.

This keeps the development experience as is but makes the built assets work on iOS and Safari.

Let me know what you think 👍

@goetzrobin goetzrobin force-pushed the webkit-browser-fix branch 2 times, most recently from 0f8ebad to bb9d5fb Compare January 30, 2023 17:09
@goetzrobin goetzrobin marked this pull request as ready for review January 30, 2023 17:09
@brandonroberts
Copy link
Member

@goetzrobin I like this option and I think that's a reasonable change

…ble with Webkit

Angular's compilation results use static blocks by default.
These static blocks are not supported by Safari or other webkit based browsers.
https://caniuse.com/mdn-javascript_classes_static_initialization_blocks
We need to use babel transformations to make the output compatible with iOS if we
want to support them.

Closes analogjs#202
@brandonroberts brandonroberts merged commit c70e5dc into analogjs:main Jan 30, 2023
@goetzrobin goetzrobin deleted the webkit-browser-fix branch February 6, 2023 14:46
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.

Analog not working in Webkit based browsers
2 participants