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

Fixed Failed Transliteration Import showing blank screen on IE11 #2545

Closed
wants to merge 4 commits into from

Conversation

rhenshaw56
Copy link
Contributor

Resolves #2535
Testing

  • Pull this branch
  • Start the app and run on Internet explorer 11

@rhenshaw56 rhenshaw56 changed the base branch from master to development July 10, 2017 16:30
@spencern
Copy link
Contributor

@rhenshaw56 I figured it would be something like this, however you've got some failing tests here that need to be resolved before we can merge.

@@ -1,4 +1,6 @@
import { slugify } from "transliteration";
// import { slugify } from "transliteration";
Copy link
Collaborator

Choose a reason for hiding this comment

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

if we are going to remove this, let's remove it, not comment it out.

@@ -1,5 +1,5 @@
import url from "url";
import { slugify } from "transliteration";
import "transliteration/lib/browser/transliteration.js";
Copy link
Collaborator

Choose a reason for hiding this comment

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

You've introduced a linting error there.

Copy link
Collaborator

Choose a reason for hiding this comment

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

That's also what's causing the tests to fail

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we might need to do something like this (this is what was there in a previous incarnation)

if (Meteor.isServer) {
  import { slugify } from "transliteration";
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

None of that is working. It looks like this has fixed the problem on the clients-side but broken it on the server-side.

I'm getting this error on the server-side whenever I start and it's also what is failing the tests

  TypeError: _slugify is not a function
      at _getSlug (lib/api/helpers.js:86:27)
      at Object.autoValue (lib/collections/schemas/products.js:467:9)
      at Object.runAV (packages/aldeed_simple-schema.js:1282:26)
      at packages/aldeed_simple-schema.js:1383:13
      at Array.forEach (native)
      at Function._.each._.forEach (packages/underscore.js:139:11)
      at packages/aldeed_simple-schema.js:1382:7

adding in any import for the server causes the client-side to fail again. I am convinced that dzcpy/transliteration#38 is related. We might want to pull in dzcpy/transliteration#44 and see if that fixes it.

@brent-hoover
Copy link
Collaborator

brent-hoover commented Jul 12, 2017

I've tested this using development and this version of transliteration and that appears to solve the problem.

@brent-hoover brent-hoover deleted the rowland-fix-issue-2535 branch July 12, 2017 13:45
@dzcpy
Copy link

dzcpy commented Jan 11, 2019

Hi guys, please check v2 version of transliteration. There are multiple improvements and it's better tested. I believe the issue is fixed. If not, just let me know.

@brent-hoover
Copy link
Collaborator

@dzcpy Thanks so much for the update. We'll update and give it a try. Would love to be back on the "mainstream" version.

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.

4 participants