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

Spread operator not transpiled #3052

Closed
hguillermo opened this issue Jun 19, 2019 · 10 comments
Closed

Spread operator not transpiled #3052

hguillermo opened this issue Jun 19, 2019 · 10 comments

Comments

@hguillermo
Copy link

I am using the latest Svelte library but I noticed a spread operator is not being transpiled. This works okay for most browsers but it is failing in MS Edge. This is the code (on_destroy.push(...new_on_destroy);):

	function mount_component(component, target, anchor) {
	    const { fragment, on_mount, on_destroy, after_render } = component.$$;
	    fragment.m(target, anchor);
	    // onMount happens after the initial afterUpdate. Because
	    // afterUpdate callbacks happen in reverse order (inner first)
	    // we schedule onMount callbacks before afterUpdate callbacks
	    add_render_callback(() => {
	        const new_on_destroy = on_mount.map(run).filter(is_function);
	        if (on_destroy) {
	            on_destroy.push(...new_on_destroy);
	        }
	        else {
	            // Edge case - component was destroyed immediately,
	            // most likely as a result of a binding initialising
	            run_all(new_on_destroy);
	        }
	        component.$$.on_mount = [];
	    });
	    after_render.forEach(add_render_callback);
	}

For now I am also including svelte in the babel configuration section to make it work. Something like this:

babel({
	exclude: /node_modules\/(?!svelte)/,
	runtimeHelpers: true,
	extensions: [".js", ".mjs", ".html", ".svelte"]
}),

I am not totally sure if there is something I can do to fix this or if we need to include a fix on Svelte. Any ideas? Thank you in advance.

@arxpoetica
Copy link
Member

Discussed this in the forum. In essence, Svelte tries to do modern-ish JS without having too strong an opinion about legacy ES. Closing as the solution listed above is what Babel is built for.

It is surprising that Edge only half supports spread. Putting this in the ticket in case one of the other devs wants to revisit: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Spread_syntax#Browser_compatibility

@pngwn
Copy link
Member

pngwn commented Jun 19, 2019

It's probably worth discussing whether MS Edge should be supported out of the box. For all its faults it is probably considered a modern browser. Or is compilation to a more widely supported ES version simply a requirement for Svelte?

@arxpoetica arxpoetica reopened this Jun 19, 2019
@hguillermo
Copy link
Author

It would be great to show in the README that Edge is not supported by default and include the code to fix it in the roll-up config. Or just transpile it. I don't see potential issues doing this. So far that is the only issue I found in Edge.

@Conduitry Conduitry added the docs label Jun 21, 2019
@antony
Copy link
Member

antony commented Jun 26, 2019

I've been maintaining a "discover as I go" IE11 and Edge compatible build here: https://github.com/antony/sapper-ie

What I'd like to do ideally is:

  1. Contribute back the config which allows Sapper to work in Edge out of the box. It's low priority because soon Edge will be Chromium and all will be well, but in terms of adoption, it's a very small change to the non-legacy build, and boils down to transpiling rest-spread.

  2. Document details about a rollup plugin (see below) which polyfills (via polyfill.io at compilation time) all the things required for IE11 support. It's actually a bundle of about 5 polyfills (I've added some extras to support third party libraries I need)

  3. Create a rollup plugin. In the repo above I've started building one in a branch which you can inspect. Including these polyfills in the legacy bundle, at compile time, would make a very clean way to get a Sapper app which works in 99% of current browsers with nearly no effort. I think from experience, this is what people would generally look for when choosing a framework.

Sadly my rollup knowledge isn't enough to figure out exactly how to get the polyfills to bundle before any other code, including shimport, so this is something I would need a pointer for.

@kylecordes
Copy link

app which works in 99% of current browsers with nearly no effort. I think from experience, this is what people would generally look for when choosing a framework

This is where the competitive bar is in 2019.

@YogliB
Copy link

YogliB commented Jul 9, 2019

app which works in 99% of current browsers with nearly no effort. I think from experience, this is what people would generally look for when choosing a framework

This is where the competitive bar is in 2019.

So, babel + browserlist set to cover 99%?

@antony
Copy link
Member

antony commented Jul 10, 2019

To be honest I think Svelte should support Edge out of the box to be a commerically viable (i.e. something you can "sell" to a business stakeholder / tech decision maker) alternative to React / Vue. It's a temporary measure because when the Chromium version of Edge is fully available then we can revert the transpilation and make it an edge case.

I love modern JS but sometimes the sensible path is the one which fosters wider adoption.

@ghost
Copy link

ghost commented Dec 16, 2019

This may all be moot now that general release of the "brand new" Edge featuring all of Chromium's guts is right around the corner: https://blogs.windows.com/msedgedev/2019/11/04/edge-chromium-release-candidate-get-ready/

@antony
Copy link
Member

antony commented Dec 16, 2019

@dkondrad yes and no. Whilst most will hopefully auto-upgrade, there will still be people stuck on Edge browsers.

Whilst Svelte is a web-project, it should work on the web. The web is unfortunately still plagued by garbage like (Legacy) Edge and IE.

Babel is a complex system which requires considerable effort and knowledge to get working. I believe full legacy Edge compatibility can be reached simply by adding a single Buble transform to the rollup/webpack config. Alternately it would be a worthwhile thing to compile away, IMHO.

@antony
Copy link
Member

antony commented Jul 4, 2020

Closing in favour of #558

@antony antony closed this as completed Jul 4, 2020
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

No branches or pull requests

7 participants