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

Use es2019 in vite config #1553

Merged
merged 5 commits into from May 31, 2023
Merged

Use es2019 in vite config #1553

merged 5 commits into from May 31, 2023

Conversation

keller-mark
Copy link
Member

@keller-mark keller-mark commented May 26, 2023

Discovered while testing the release PR v3.0.0 with portal-ui

Background

In testing the release PR v3.0.0 with portal-ui i ran into this issue that the current Vitessce bundle contains nullish coalescing syntax which the portal-ui webpack config does not support (though modern browsers do). I could not figure out a way to update the portal's webpack/babel/browserslist configs to make it work, so i think for now the easiest workaround is for us to stick to outputting ES2019 JS in Vitessce until the portal updates its bundling infrastructure.

I thought i might be able to get babel (in portal-ui) to transform the code https://babeljs.io/docs/babel-plugin-transform-nullish-coalescing-operator but that didnt seem to work, even when opting node_modules/vitessce out of the babel-loader exclusion list. These issues that suggest webpack 4 is not compatible with nullish coalescing:

Screen Shot 2023-05-26 at 2 37 02 PM

With the changes in this PR, it resolves the error:
Screen Shot 2023-05-26 at 3 40 27 PM

Once resolved, we should also make an issue to track this here or in portal-ui repo to be able to remove this build target restriction at some point in the future

Change List

Checklist

  • Ensure PR works with all demos on the dev.vitessce.io homepage
  • Open (draft) PR's into vitessce-python and vitessce-r if this is a release PR
  • Documentation added or updated

@@ -14,6 +14,7 @@ export default defineConfig({
emptyOutDir: false,
minify: isProduction ? 'esbuild' : false,
sourcemap: false,
target: 'es2019', // portal-ui cannot handle the nullish coalescing output by ESNext
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this a reversion? i don't know enough about es versions.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure what you mean by reversion - the Vite docs say the default target is 'modules' which is effectively es2020

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So it is going backwards in a way but it should not have an effect on the functionality.

I think it is possible that perhaps there will be new syntax in future es targets that we would not want to be transpiled away / might not be possible to use under an earlier target. For this reason, we should still track the status of the portal-ui bundling setup to potentially remove or bump the target in the future, and we can let the portal-ui developers know

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I'm fine with that. As long as they know. I'm sure upgrading to v5 would have benefits.

@keller-mark
Copy link
Member Author

I think this is the last fix that we should get in for the release, at least pending anything new we discover upon testing with portal-ui during testing once it is merged into main / the release PR #1548

@keller-mark keller-mark merged commit 4f80119 into main May 31, 2023
5 checks passed
@keller-mark keller-mark deleted the keller-mark/portal-es2019 branch May 31, 2023 14:48
@keller-mark keller-mark mentioned this pull request Sep 11, 2023
11 tasks
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 this pull request may close these issues.

None yet

2 participants