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

Advanced Editing Demo fails with console errors in IE11 #1228

Closed
blzaugg opened this issue Apr 19, 2019 · 9 comments · Fixed by #1236
Closed

Advanced Editing Demo fails with console errors in IE11 #1228

blzaugg opened this issue Apr 19, 2019 · 9 comments · Fixed by #1236

Comments

@blzaugg
Copy link

blzaugg commented Apr 19, 2019

Do you want to request a feature or report a bug?
Bug

What is the current behavior?
Demo doesn't work and console errors in browser

SCRIPT1002: Syntax error
File: alloy-editor-all.js, Line: 36086, Column: 21
SCRIPT5009: 'AlloyEditor' is undefined
File: localhost:9000, Line: 109, Column: 13

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem.

  1. Following the steps from Build it
$ yarn install
$ yarn build
$ yarn start
  1. Open http://localhost:9000/ in IE 11

What is the expected behavior?
Demo works with no console errors.

Which versions of alloy-editor, and which browser / OS are affected by this issue? Did this work in previous versions?
Windows 7 Internet Explorer 11

@blzaugg
Copy link
Author

blzaugg commented Apr 19, 2019

Advanced Editing Demo IE 11

@blzaugg
Copy link
Author

blzaugg commented Apr 19, 2019

The syntax error is from an arrow function that isn't transpiled to ES5:

module.exports = () => {
	return {
		code: `module.exports = ${JSON.stringify(version)};`,
	};
};

@blzaugg
Copy link
Author

blzaugg commented Apr 19, 2019

/cc @gregory-bretall

@wincent
Copy link
Contributor

wincent commented Apr 22, 2019

Thanks for reporting this @blzaugg!

The syntax error is from an arrow function that isn't transpiled to ES5:

I'm not sure if that's the function though; that file should only be evaluated as part of the build process, and should never wind up in the browser. We'll dig into it further though.

@wincent
Copy link
Contributor

wincent commented Apr 23, 2019

I'm afraid I can't repro this on IE 11 @blzaugg. Any ideas what might be needed to trigger? (I'm using IE 11 in a VirtualBox VM, hitting the webpack dev server that you get with yarn start on port 9000.

Screen Shot 2019-04-23 at 16 53 42

@gregory-bretall
Copy link
Contributor

@wincent I was able to reproduce this on the most up to date master branch from today of alloy-editor, just by using yarn start. Not quite sure what the difference here is unless it's testing IE11 on Windows 10 potentially?

@wincent
Copy link
Contributor

wincent commented Apr 23, 2019

Ok, thanks for that @gregory-bretall. I tested on a Windows machine here and I see the fix that is required. It's a build-time fix and not a run-time fix — on Windows the build produces a bad result.

The reason I couldn't repro in the VM is that I wasn't running the build in Windows; I was running the build on Mac and hitting that from Windows.

@gregory-bretall
Copy link
Contributor

That makes sense then, thanks for looking into it @wincent!

wincent added a commit that referenced this issue Apr 23, 2019
It turns out that the "test" property in the Webpack build needs to be
in a different format on Windows, otherwise it never matches, which
means that our file at "scripts/build/version.js" doesn't pass through
the val-loader, and in turn means that we produce untransformed output
that IE 11 will choke on (an arrow function, and even if we fixed it, it
would probably choke on the backticks).

You can make the build work on Windows by passing
`path.resolve('scripts/build/version.js')` instead; this produces an
absolute path with the appropriate Windows-style backslashes. But then
the build doesn't work on macOS. If you try to make it a relative path
(`path.relative('', path.resolve(...))`) then Webpack rejects it.

So, the solution is to make a RegExp with the backslashes (if present)
escaped.

Test plan: build on macOS and Windows (with `yarn build`, `yarn start`)
and confirm console errors are gone and version is correctly transpiled.
Note that the demo misbehaves still on IE 11 (not console errors, but
some misbehavior of the editor itself, but I think that's expected).

Closes: #1228
wincent added a commit that referenced this issue Apr 23, 2019
@blzaugg
Copy link
Author

blzaugg commented Apr 23, 2019

Thanks @gregory-bretall

julien added a commit that referenced this issue Apr 24, 2019
fix: console errors in Windows-based build (#1228)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants