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

Fix the build configurations to upgrade Streamlit to v1.17 #471

Closed
whitphx opened this issue Feb 6, 2023 · 2 comments · Fixed by #472
Closed

Fix the build configurations to upgrade Streamlit to v1.17 #471

whitphx opened this issue Feb 6, 2023 · 2 comments · Fixed by #472

Comments

@whitphx
Copy link
Owner

whitphx commented Feb 6, 2023

When I tried to upgrade the streamlit submodule to v1.17 (rebasing the latest branch stlite-1.13.0 to 1.17.0), the build was broken.
For example, when running yarn start in @stlite/mountable, the following error occurs.

Failed to compile.

/path/to/stlite/streamlit/frontend/src/components/widgets/Slider/Slider.tsx 78:39
Module parse failed: Unexpected token (78:39)
File was processed with these loaders:
 * ../../node_modules/@pmmmwh/react-refresh-webpack-plugin/loader/index.js
 * ./node_modules/babel-loader/lib/index.js
You may need an additional loader to handle the result of these loaders.
|       const thumbIndex = $thumbIndex || 0;
|       this.thumbRef[thumbIndex] = ref;
>       this.thumbValueRef[thumbIndex] ||= /*#__PURE__*/React.createRef();
|       const formattedValue = $value ? this.formatValue($value[$thumbIndex]) : "";
|       const passThrough = pick(props, ["role", "style", "aria-valuemax", "aria-valuemin", "aria-valuenow", "tabIndex", "onKeyUp", "onKeyDown", "onMouseEnter", "onMouseLeave", "draggable"]);

The line this error is pointing to has been introduced at https://github.com/streamlit/streamlit/pull/5913/files#diff-845917f3a07167e741db444532fae1e083d5b9f84ac8e8e38d3a34818a311ad8R242, where the logical assignment operators were started being used. This syntax is at Stage 4 (https://github.com/tc39/proposal-logical-assignment).

@whitphx
Copy link
Owner Author

whitphx commented Feb 6, 2023

This can be solved by setting the "browserslist" to be same as the streamlit frontend package.

-  "browserslist": {
-    "production": [
-      ">0.2%",
-      "not dead",
-      "not op_mini all"
-    ],
-    "development": [
-      "last 1 chrome version",
-      "last 1 firefox version",
-      "last 1 safari version"
-    ]
-  },
+  "browserslist": [
+    ">0.2%",
+    "not dead",
+    "not ie <= 11",
+    "not op_mini all"
+  ],

This field controls the Babel transpilation target (it is configured for CRA here), and this change makes the output in dev-mode compatible older browsers by adding >0.2% specification to the "development" option.

It implies that with the current browserslist config, Babel emits such modern syntax without transpiling it, and some successor step in the Webpack build process raises the error as it can't parse it.
If it's true, it's a CRA's bug because it does not work well with some browserlists settings, however, we still have to solve or work around it anyway.


So, the solutions would be as follow.

  1. Configure the browserslist settings. This is a workaround, and can lead to larger bundle-size because some modern syntaxes have to be converted to old redundant representations.
  2. Upgrade react-scripts to the latest version. This could be a fundamental solution. However it also need to be applied to streamlit submodule, so syncing it to the upstream may be harder to do. If we can merge this change into the upstream, there would be no problem and it would be the most clean solution, but there is no incentive for the upstream dev team to accept it.

@whitphx
Copy link
Owner Author

whitphx commented Feb 6, 2023

I found another simple workaround, adding a plugin specific for that syntax, @babel/plugin-proposal-logical-assignment-operators in this case, to the babel.plugins field in craco.config.js

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

Successfully merging a pull request may close this issue.

1 participant