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

Upgraded es-module-shims to v1.5.5 #128

Merged
merged 1 commit into from May 22, 2022

Conversation

iainbeeston
Copy link
Contributor

The current version of es-module-shims provided
by importmap-rails (v1.4.6) requires allowing
frame-src: blob: in an app's CSP policy (it
creates an iframe with a blob as it's source
in order to perform feature detection). This is
quite a permissive thing to allow in csp and
I wouldn't recommend it if there is an alternative.

Since v1.4.7 es-module-shims no longer uses a
blob and it will work with script-src: 'self'
instead. I've upgraded to the latest version,
which is v1.5.5. For more details about the
CSP issue and it's fix see:

The current version of es-module-shims provided
by importmap-rails (v1.4.6) requires allowing
`frame-src: blob:` in an app's CSP policy (it
creates an iframe with a blob as it's source
in order to perform feature detection). This is
quite a permissive thing to allow in csp and
I wouldn't recommend it if there is an alternative.

Since v1.4.7 es-module-shims no longer uses a
blob and it will work with `script-src: 'self'`
instead. I've upgraded to the latest version,
which is v1.5.5. For more details about the
CSP issue and it's fix see:

* guybedford/es-module-shims#265
* guybedford/es-module-shims#290
@iainbeeston
Copy link
Contributor Author

iainbeeston commented May 16, 2022

I've assumed that a minor version upgrade (from 1.4.x to 1.5.x) won't be a problem as this is a shim and isn't directly called by the gem (AFAIK)

@dhh dhh merged commit 9696db2 into rails:main May 22, 2022
@crohr
Copy link

crohr commented Jun 2, 2022

Hello,

We are experiencing issues after upgrading to import map-rails 1.1.0 from 1.0.3: with a blank Rails 7.0.3 app and the default HelloWorld stimulus controller, everything works fine on Desktop and Mobile Safari. However on iOS the JS isn't loaded from a React Native WebView, whereas it used to work fine before. No errors are present in the Safari developer console.

I suppose this comes from a specific behaviour of the React Native WebView component, so probably nothing that affects too many people, but just in case you'd like to have a look, I've setup a minimal reproducible case at: https://github.com/crohr/test-importmap

@dhh
Copy link
Member

dhh commented Jun 2, 2022

Cc @guybedford ring any bells?

@guybedford
Copy link
Contributor

There was work on fixing some of the load event behaviours in guybedford/es-module-shims#287, which may well be the cause. One way to verify this would be to try ES Module Shims 1.5.4 in the application and see if that resolves the issue to narrow it down.

It would help to have a single HTML page replication. I don't have a react native testing environment to hand so any better steps for replicating or investigating react native timing issues would be appreciated.

@crohr
Copy link

crohr commented Jun 3, 2022

@guybedford Thanks for the follow-up, I'll attempt to use ES Module Shims 1.5.4 and report if that works.

@crohr
Copy link

crohr commented Jun 3, 2022

@guybedford the issue actually started with v1.4.7 (v1.5.x are all broken), whereas I can confirm that it worked until v1.4.6. The diff doesn't look too big but there is a mention in the changelog regarding a fix for Android and iframe, so could be related?

When it works, we get this message:

OK: ^ TypeError module failure has been polyfilled

Whereas it is absent when it's broken.

@guybedford
Copy link
Contributor

@crohr thanks for bisecting further here - it does sound like it is likely that one line diff in src/features.js then. I've posted a fix in guybedford/es-module-shims#298 since it is actually ok to revert that now for other reasons. If you're able to verify that diff before we land that would be a great help.

@guybedford
Copy link
Contributor

I've published a 1.5.6 that should solve this issue if the bisection was correct!

@crohr
Copy link

crohr commented Jun 4, 2022

@guybedford I can confirm that v1.5.6 works in React Native Webview on iOS. Thanks a lot!

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.

None yet

4 participants