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

Bundle contains non IE11 compatible code #6502

Closed
hallsbyra opened this issue Jun 22, 2021 · 11 comments
Closed

Bundle contains non IE11 compatible code #6502

hallsbyra opened this issue Jun 22, 2021 · 11 comments

Comments

@hallsbyra
Copy link
Contributor

🐛 bug report

Bundle contains lots of non IE11 compatible code, even with "browserslist": ["IE 11"].

🎛 Configuration (.babelrc, package.json, cli command)

Empty project, just a dummy index.html + index.js.

package.json
"browserslist": ["IE 11"]

CLI
npx parcel serve .\index.html

🤔 Expected Behavior

Bundle should run in IE11.

😯 Current Behavior

Syntax error in IE11. Bundle contains multiple non IE11-code. This is some injected boilerplate code and does not originate from my simple index.js.

Some examples from the bundle:

IE doesn't like the trailing comma:

      modules[name][0].call(
        module.exports,
        localRequire,
        module,
        module.exports,
        this,
      );

Arrow function:

let assets = data.assets.filter((asset)=>asset.envHash === HMR_ENV_HASH

IE doesn't support for of:

for (let ansiDiagnostic of data.diagnostics.ansi){

💁 Possible Solution

Bundle should be free of non IE11. If this is injected boilerplate, that code should be compatible with IE11. Either by changing the boilerplate template or by running it through some transpilation.

🔦 Context

N/A

💻 Code Sample

I think my examples above are enough to find the issue. If not, I'm happy to push a simple repro.

🌍 Your Environment

Software Version(s)
Parcel @next
Node 16
npm/Yarn 7.13
Operating System Win10
@mischnic
Copy link
Member

mischnic commented Jun 22, 2021

The files in questions:

https://github.com/parcel-bundler/parcel/blob/v2/packages/runtimes/hmr/src/loaders/hmr-runtime.js

They aren't transpiled, so these few cases needs to be fixed manually

@danieltroger
Copy link
Contributor

This is like the third issue about this lol. But this only happens when not using scope hoisting, right? Because I deployed my production code from an SWC version today…

@mischnic
Copy link
Member

The first is only with HMR, the second is only without scope hoisting, the third is with scopehoisting.

But running terser should remove trailing commas anyway

@danieltroger
Copy link
Contributor

Ah, perfect

@hallsbyra
Copy link
Contributor Author

I started cleaning out the mentioned files, but then I saw that the bundle also includes plenty of dynamically generated arrow functions that look like this:

parcelHelpers.export(exports, "ObservableMap", ()=>ObservableMap
);
parcelHelpers.export(exports, "ObservableSet", ()=>ObservableSet
);
parcelHelpers.export(exports, "Reaction", ()=>Reaction
);
parcelHelpers.export(exports, "_allowStateChanges", ()=>allowStateChanges
);
parcelHelpers.export(exports, "_allowStateChangesInsideComputed", ()=>runInAction
);
parcelHelpers.export(exports, "_allowStateReadsEnd", ()=>allowStateReadsEnd
);

Don't know how to proceed with those.

@mischnic
Copy link
Member

That comes from here:

Expr::Arrow(ArrowExpr {

This change should fix that:

diff --git packages/transformers/js/core/src/modules.rs packages/transformers/js/core/src/modules.rs
index e03b7145d..f6d0360f3 100644
--- packages/transformers/js/core/src/modules.rs
+++ packages/transformers/js/core/src/modules.rs
@@ -169,14 +169,26 @@ impl ESMFold {
           kind: StrKind::Synthesized,
           span: DUMMY_SP,
         })),
-        Expr::Arrow(ArrowExpr {
-          body: BlockStmtOrExpr::Expr(Box::new(local)),
-          is_async: false,
-          is_generator: false,
-          params: vec![],
-          span: DUMMY_SP,
-          return_type: None,
-          type_params: None,
+        Expr::Fn(FnExpr {
+          ident: None,
+          function: Function {
+            body: Some(BlockStmt {
+              span: DUMMY_SP,
+              stmts: vec![Stmt::Return({
+                ReturnStmt {
+                  span: DUMMY_SP,
+                  arg: Some(Box::new(local)),
+                }
+              })],
+            }),
+            is_async: false,
+            is_generator: false,
+            params: vec![],
+            decorators: vec![],
+            span: DUMMY_SP,
+            return_type: None,
+            type_params: None,
+          },
         }),
       ],
       span,

Feel free to open a PR with the cases you already fixed though, if you don't want to change/compile the Rust code


Two more cases which only happen in production builds are here:

)}, () => ${resolved}${
asset.meta.hasCJSExports ? `, (v) => ${resolved} = v` : ''

Currently, you should see something like

$parcel$export(module.exports, "Thing", () => $92956f215d0421a7$export$62d01221e08cc378);

when doing dynamic imports or if shared bundles are created.

@hallsbyra
Copy link
Contributor Author

Ok, submitting what I've done so far. There's a clash with yarn format which wants to re-introduce trailing commas into the files.

Also I couldn't reproduce the last two cases (with dynamic imports/shared bundles), so did not include them.

@danieltroger
Copy link
Contributor

I also found this. Does it go away when setting my target to something different / is it covered by this issue or should I open another one?

IE11 (which this was built for) I think technically supports let but normally it's transpiled to var everywhere.

Screenshot 2021-07-19 at 16 56 55

Screenshot 2021-07-19 at 16 56 41

@danieltroger
Copy link
Contributor

The let issue also happens in chrome 44. I tested it and when building for chrome 44 it also includes the let. This is sad to see because it means all compatibility is currently broken. Is there some info when this regression was introduced? Was it part of the SWC update?

Could running the parcel output through babel be a possible fix?

@devongovett
Copy link
Member

Should be fixed by #6532

@newtriks
Copy link

@devongovett this does not seem to be fixed sadly. I'll get a reproduction in place but this open thread (there are others related to non-transpilation) but this is the clearest) basically shows clear examples #6676 and evidence of the issue still remaining for IE 11.

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

No branches or pull requests

5 participants