Skip to content

Fix es5 build#7319

Merged
bramkragten merged 6 commits intodevfrom
fix-es5-build
Oct 14, 2020
Merged

Fix es5 build#7319
bramkragten merged 6 commits intodevfrom
fix-es5-build

Conversation

@bramkragten
Copy link
Member

Proposed change

Fixes our ES5 build

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New feature (thank you!)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Example configuration

Additional information

  • This PR fixes or closes issue: fixes #
  • This PR is related to issue:
  • Link to documentation pull request:

Checklist

  • The code change is tested and works locally.
  • There is no commented out code in this PR.
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:


<script type="module" crossorigin="use-credentials">
import "<%= latestPageJS %>";
<script crossorigin="use-credentials">
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need this PR any more? #7318

Copy link
Member Author

Choose a reason for hiding this comment

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

Nope that's all in this one

// Only support the syntax, Webpack will handle it.
"@babel/plugin-syntax-import-meta",
"@babel/syntax-dynamic-import",
"@babel/plugin-syntax-dynamic-import",
Copy link
Member

Choose a reason for hiding this comment

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

We used to only load syntax because webpack dealt with the processing. What do we gain from adding this?

Copy link
Member Author

Choose a reason for hiding this comment

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

Wasn't this just a typo?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is still the plugin for the syntax

rules: [
{
test: /\.js$|\.ts$/,
test: /\..?js$|\.ts$/,
Copy link
Member

Choose a reason for hiding this comment

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

Let's make the m more explicit. Think that this should work?

Suggested change
test: /\..?js$|\.ts$/,
test: /\.m?js$|\.ts$/,

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, made it a point so it would also pick up cjs and I don't know what other types of js files are out there.

@bramkragten bramkragten merged commit f6ff652 into dev Oct 14, 2020
@bramkragten bramkragten deleted the fix-es5-build branch October 14, 2020 20:20
@bramkragten bramkragten mentioned this pull request Oct 21, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Jul 5, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants