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.

Sign up for GitHub

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

Jump to bottom

Use shared Babel.js configuration #27022

Closed
wants to merge 1 commit into from
Closed

Use shared Babel.js configuration #27022

wants to merge 1 commit into from

Conversation

francoisfreitag
Copy link
Contributor

Keep in sync with other repositories by using the centralized
configuration for Babel.js.

@francoisfreitag francoisfreitag mentioned this pull request May 19, 2021
Merged
@francoisfreitag francoisfreitag force-pushed the use-common-config branch 2 times, most recently from 9c24802 to 4729d6c Compare May 19, 2021 07:46
@francoisfreitag
Copy link
Contributor Author

I regenerated the JS and source map files after the config change, hopefully that’ll make the Node build green.

@francoisfreitag
Use shared Babel.js configuration
dd90d9b 
Keep in sync with other repositories by using the centralized
configuration for Babel.js.

Signed-off-by: François Freitag <mail@franek.fr>
@francoisfreitag
Copy link
Contributor Author

Figured it out, I had make watch-js running in another tab 🤦.

@francoisfreitag
Copy link
Contributor Author

I do not understand what is going on yet, but it is due to the switch from modules: false to modules: 'commonjs'.
I can reproduce the failure locally, will get more familiar with the karma test setup and hopefully report back within a few days.

@francoisfreitag
Copy link
Contributor Author

Changing the config is a large change.

Previously, because module was false, the “compiled” JavaScript could execute directly in the global context. When modules is set to 'commonjs', the scripts are limited to their own module. The globals should likely be registered through the ProvidePlugin.
I’ll investigate how large of a change that is and report back.

@skjnldsv
Copy link
Member

I do not understand what is going on yet, but it is due to the switch from modules: false to modules: 'commonjs'.
I can reproduce the failure locally, will get more familiar with the karma test setup and hopefully report back within a few days.

Ah, yes, unfortunately our test suite might not be compatible with our babel config. It's not a trivial change.
I started addressing this in #23973, but it's a mess.

@skjnldsv skjnldsv added this to the Nextcloud 23 milestone May 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@artonge artonge artonge approved these changes

Assignees
No one assigned
Labels
2. developing Work in progress enhancement feature: dependencies
Projects
None yet
Milestone
Nextcloud 23
Development

Successfully merging this pull request may close these issues.
3 participants
@francoisfreitag @skjnldsv @artonge