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

IE 11 support #1525

Merged
merged 1 commit into from Sep 18, 2020
Merged

IE 11 support #1525

merged 1 commit into from Sep 18, 2020

Conversation

@benmccann
Copy link
Contributor

@benmccann benmccann commented Sep 17, 2020

Closes #1505

Upgrades to the latest version of shimport including the fix: Rich-Harris/shimport#32

@benmccann benmccann added the bundling label Sep 17, 2020
@Conduitry
Copy link
Member

@Conduitry Conduitry commented Sep 17, 2020

Is the inject_styles file not getting sent through Babel (or, for that matter, Terser)? Is it just being output as-is, untranspiled and unminified?

@benmccann
Copy link
Contributor Author

@benmccann benmccann commented Sep 17, 2020

On master yes. But I've sent #1524 to fix that

@Conduitry
Copy link
Member

@Conduitry Conduitry commented Sep 17, 2020

Should this be blocked by that then? It looks like several of the changes here are for rewriting in older JS syntax, unrelated to the import.meta.url thing.

@benmccann benmccann marked this pull request as draft Sep 18, 2020
@Conduitry
Copy link
Member

@Conduitry Conduitry commented Sep 18, 2020

Once the inject_styles script is being transpiled and minified correctly via #1524, and once Rich-Harris/shimport#32 is merged and released, is there any of this PR we still need besides updating shimport?

@benmccann benmccann force-pushed the benmccann:chrome-54 branch from ea3236a to eec0e50 Sep 18, 2020
@benmccann benmccann changed the title Chrome 54 support IE 11 support Sep 18, 2020
@benmccann
Copy link
Contributor Author

@benmccann benmccann commented Sep 18, 2020

#1524 does minify the inject_styles output, but it doesn't seem to transpile it. I would assume if it's getting sent through terser it must be sent through babel as well, so I'm not quite sure what's happening. It's not that many bytes though and I've already burnt so much time on this that I'm not sure it's really worth figuring out - though I do agree it'd be slightly nicer

For line 133, renderDynamicImport happens after transform, so I think that won't be able to be transformed by Babel and that change to function there is necessary

@antony
Copy link
Member

@antony antony commented Sep 18, 2020

@benmccann the newest Babel is completely weird, it uses the filename of the babel config to determine what it applies to. I believe we should copy the babel config from the IE project into the official template, and it will fix this, because I literally spent weeks figuring it out.

@benmccann benmccann force-pushed the benmccann:chrome-54 branch from eec0e50 to 4449338 Sep 18, 2020
@benmccann benmccann marked this pull request as ready for review Sep 18, 2020
@benmccann benmccann force-pushed the benmccann:chrome-54 branch from 4449338 to 6b1730b Sep 18, 2020
@benmccann
Copy link
Contributor Author

@benmccann benmccann commented Sep 18, 2020

Probably a good idea to copy your config into the template.

That's a bit different than the issue here though because I've been doing all my testing against antony/sapper-ie and this code still isn't being transpiled. I see the code on the filesystem getting transpiled. Maybe it's some kind of issue with the Rollup Babel plugin where it won't transpile code that doesn't exist on the filesystem or something. I don't really think it's worth spending several hours trying to figure it out though since this works

@benmccann benmccann merged commit bb05226 into sveltejs:master Sep 18, 2020
10 checks passed
10 checks passed
Tests (10, ubuntu-latest)
Details
Tests (10, windows-latest)
Details
Tests (10, macOS-latest)
Details
Tests (12, ubuntu-latest)
Details
Tests (12, windows-latest)
Details
Tests (12, macOS-latest)
Details
Tests (14, ubuntu-latest)
Details
Tests (14, windows-latest)
Details
Tests (14, macOS-latest)
Details
Lint
Details
@antony
Copy link
Member

@antony antony commented Sep 21, 2020

@benmccann I've finally had a chance to try this build and I'm afraid it doesn't work.

The reason is that the shimport file is an untranspiled file in the /client/ directory, so IE11 just balks at the first const.

Screenshot from 2020-09-21 17-55-51

I'm going to see what happened with the older version - I feel like I remember that shimport was transpiled and loaded from legacy, but I don't exactly remember.

@antony antony mentioned this pull request Sep 21, 2020
@antony
Copy link
Member

@antony antony commented Sep 21, 2020

I've raised #1544 to track

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

3 participants
You can’t perform that action at this time.