Skip to content

Fix ie loading bug#395

Closed
dannyjlaurence-exp wants to merge 2 commits intofullstackreact:masterfrom
TeamEngineIO:master
Closed

Fix ie loading bug#395
dannyjlaurence-exp wants to merge 2 commits intofullstackreact:masterfrom
TeamEngineIO:master

Conversation

@dannyjlaurence-exp
Copy link
Copy Markdown

Closes #365

This PR has two things:

  • Fix script bug
  • Use yarn instead of npm
  • Replace "." with "bash" to better support cygwin
  • Checking in the result of building? I was kind of confused when building produced different results in the dist/folder

The specific fix for the loaderCB bug was pretty straightforward: The handleResult callback in the ScriptCache.js was being used both onLoad and as the callback to the Google script being loaded. On Chrome, the google callback runs first reliably and in IE/Edge onLoad runs first reliably - so in IE the window.loaderCB function that the Google Script was trying to call was already cleaned up.

To fix, I just naively sent a different named state to the handleResult function, and also returned on resolve and reject. I'm a little unclear on if this will cause any other issues - but for what we use this project for - it works fine.

* Fix script bug

* Use yarn instead of npm
Comment thread Makefile

dev:
npm run dev
yarn dev
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Let's keep this with npm as it's more universal still at this stage.

Comment thread package.json
"lint": "eslint ./src",
"lintfix": "eslint ./src --fix",
"testonly": "NODE_ENV=test mocha $npm_package_options_mocha",
"testonly": "NODE_ENV=test mocha --require scripts/mocha_runner -t rewireify src/**/__tests__/**/*.js",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why are we using rewireify here?

Copy link
Copy Markdown
Contributor

@auser auser left a comment

Choose a reason for hiding this comment

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

Left some feedback.

@dannyjlaurence-exp
Copy link
Copy Markdown
Author

Thanks! I missed this email. I'll follow your feedback soon.

@auser
Copy link
Copy Markdown
Contributor

auser commented Apr 21, 2020

Merged and published

@auser auser closed this Apr 21, 2020
@BenSwennen
Copy link
Copy Markdown

Hi, it looks like this https://github.com/fullstackreact/google-maps-react/pull/395/files#diff-f7444d992b0a47df772a034af14b92648da3121d4a9af3fd58fd2b074546916dR76 never got published? Or am I missing something? I'm still getting the loaderCBXXXX error on IE and Edge on version 2.0.6.

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.

Js error for ScriptCache

3 participants