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

Target es6 for esm build #378

Closed
canadaduane opened this issue Jul 3, 2022 · 7 comments
Closed

Target es6 for esm build #378

canadaduane opened this issue Jul 3, 2022 · 7 comments
Labels
support User support and general help

Comments

@canadaduane
Copy link

canadaduane commented Jul 3, 2022

Description of the bug

Previously, I could use postprocessing in my webpack bundling without any additional processing (e.g. babel). Recently, however, it appears the language target has moved to "esnext" which includes language features such as "??" and "?." which are less compatible. Specifically, the build/postprocessing.esm.js generated file includes these esnext features.

To Reproduce

Run pnpm build and inspect the build/postprocessing.esm.js file to see that it includes ?? and ?. operators.

Expected behavior

Produce a plain "es6" target.

Library versions used

  • Three: n/a
  • Post Processing: main

Potential Solution

The https://github.com/pmndrs/postprocessing/blob/main/esbuild.mjs#L69 instructions do not include target: "es6", but I believe adding this would solve the issue.

@vanruesc
Copy link
Member

vanruesc commented Jul 3, 2022

Thanks for pointing this out!

I'm ok with changing the target to es6 for now, but I'm wondering if that's really the best solution. What kind of compatibility issues are you running into specifically? Browser support for ?? is at 89% and ?. at 90% which seems good enough. ES6 classes are at 94% for comparison.

I think that compatibility concerns with devices/browsers should be left to the end user. Some transpilations of native ESNext features might produce less efficient, inflated or slower code when targeting older language versions and I'd like to avoid that.

@canadaduane
Copy link
Author

It looks like it's coming from my webpack source-map-loader step:

ERROR in ../node_modules/.pnpm/postprocessing@6.28.1_three@0.136.0/node_modules/postprocessing/build/postprocessing.esm.js 1290:136
Module parse failed: Unexpected token (1290:136)
File was processed with these loaders:
 * ../node_modules/.pnpm/source-map-loader@1.1.3_webpack@4.46.0/node_modules/source-map-loader/dist/cjs.js
You may need an additional loader to handle the result of these loaders.
|   }
|   setShaderParts(shaderParts) {
>     this.fragmentShader = effect_default.replace(EffectShaderSection.FRAGMENT_HEAD, shaderParts.get(EffectShaderSection.FRAGMENT_HEAD) ?? "").replace(EffectShaderSection.FRAGMENT_MAIN_UV, shaderParts.get(EffectShaderSection.FRAGMENT_MAIN_UV) ?? "").replace(EffectShaderSection.FRAGMENT_MAIN_IMAGE, shaderParts.get(EffectShaderSection.FRAGMENT_MAIN_IMAGE) ?? "");
|     this.vertexShader = effect_default2.replace(EffectShaderSection.VERTEX_HEAD, shaderParts.get(EffectShaderSection.VERTEX_HEAD) ?? "").replace(EffectShaderSection.VERTEX_MAIN_SUPPORT, shaderParts.get(EffectShaderSection.VERTEX_MAIN_SUPPORT) ?? "");
|     this.needsUpdate = true;
 @ ./src/ecs/plugins/core/Presentation.ts 2:0-168 21:32-46 24:32-46 26:34-44 28:33-46 29:27-40 38:31-51 39:27-40 45:38-48 50:38-48 50:86-96 51:24-34 52:35-52
 @ ./src/ecs/plugins/core/index.ts
 @ ./src/main/effects/createECSWorld.ts
 @ ./src/main/Program.ts
 @ ./src/main/index.ts
 @ multi ./styles/index.scss ./src/main/index.ts

Maybe postprocessing is on the leading edge? I believe it's the only package that has this issue in the 1500+ that our project depends on. (Which is not meant as a call-out! I'm still trying to understand the issue itself.)

@canadaduane
Copy link
Author

canadaduane commented Jul 3, 2022

Note: If I remove the source-map-loader by commenting it out:

      // Include source maps from node modules
      // {
      //   test: /\.js$/,
      //   enforce: "pre",
      //   use: ["source-map-loader"],
      // },

I get a new but similar error:

ERROR in /home/duane/tmp/postprocessing/build/postprocessing.mjs 1290:136
Module parse failed: Unexpected token (1290:136)
You may need an appropriate loader to handle this file type, currently no loaders are configured to process this file. See https://webpack.js.org/concepts#loaders
|   }
|   setShaderParts(shaderParts) {
>     this.fragmentShader = effect_default.replace(EffectShaderSection.FRAGMENT_HEAD, shaderParts.get(EffectShaderSection.FRAGMENT_HEAD) ?? "").replace(EffectShaderSection.FRAGMENT_MAIN_UV, shaderParts.get(EffectShaderSection.FRAGMENT_MAIN_UV) ?? "").replace(EffectShaderSection.FRAGMENT_MAIN_IMAGE, shaderParts.get(EffectShaderSection.FRAGMENT_MAIN_IMAGE) ?? "");
|     this.vertexShader = effect_default2.replace(EffectShaderSection.VERTEX_HEAD, shaderParts.get(EffectShaderSection.VERTEX_HEAD) ?? "").replace(EffectShaderSection.VERTEX_MAIN_SUPPORT, shaderParts.get(EffectShaderSection.VERTEX_MAIN_SUPPORT) ?? "");
|     this.needsUpdate = true;
 @ ./src/ecs/plugins/core/Presentation.ts 2:0-168 21:32-46 24:32-46 26:34-44 28:33-46 29:27-40 38:31-51 39:27-40 45:38-48 50:38-48 50:86-96 51:24-34 52:35-52
 @ ./src/ecs/plugins/core/index.ts
 @ ./src/main/effects/createECSWorld.ts
 @ ./src/main/Program.ts
 @ ./src/main/index.ts
 @ multi ./styles/index.scss ./src/main/index.ts

EDIT: Please excuse the ".mjs" extension there, I am now experimenting with pnpm link and a local copy of postprocessing.

@canadaduane
Copy link
Author

canadaduane commented Jul 4, 2022

Here is a minimum reproducible sample:

https://github.com/canadaduane/pptest

To reproduce:

pnpm i
pnpm build

EDIT: Removed typescript dependency from repro.

@canadaduane
Copy link
Author

I just did a little more testing:

Version 6.27.0 is the last working version for me. "Module parse failed" occurs in the sample pptest project if 6.28.0 or 6.28.1 is used as a dependency.

@vanruesc
Copy link
Member

vanruesc commented Jul 4, 2022

Thanks for the repro 👍

I did some research and found that support for ?? and ?. was added to acorn in version 7. Webpack 4, however, is locked to version 6 and hence doesn't support this syntax.

You should upgrade to webpack 5 if possible. I'll set the build target to es2019 which will transpile the ?? and ?. syntax to ease the transition. but this will be reverted in the next major release.

@vanruesc vanruesc added the support User support and general help label Jul 4, 2022
@canadaduane
Copy link
Author

Thanks! I had forgotten that Webpack is dependent on acorn. That was the piece I was missing in being able to explain to myself what was going on.

I believe our project can be updated to Webpack 5, but it will be a few months, so a little more time would be greatly appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
support User support and general help
Projects
None yet
Development

No branches or pull requests

2 participants