Skip to content

refactor(client): make use of ESM instead of CJS - #1967

Merged
hiroppy merged 3 commits into
nextfrom
feature/use-esm
Jun 5, 2019
Merged

refactor(client): make use of ESM instead of CJS#1967
hiroppy merged 3 commits into
nextfrom
feature/use-esm

Conversation

@hiroppy

Copy link
Copy Markdown
Contributor
  • This is a bugfix
  • This is a feature
  • This is a code refactor
  • This is a test update
  • This is a docs update
  • This is a metadata update

For Bugs and Features; did you add new tests?

no

Motivation / Use-Case

related #1950

Breaking Changes

no

Additional Info

import {
clear as clearOverlay,
showMessage as showMessageOverlay,
} from './overlay';

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

* as overlay is prohibited by eslint.

alexander-akait
alexander-akait previously approved these changes Jun 4, 2019

@alexander-akaitalexander-akait left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good, I hope we did not break something for other developers

@hiroppy

hiroppy commented Jun 4, 2019

Copy link
Copy Markdown
ContributorAuthor

I checked using samples and there was no problem. Let's see how it goes for a while.

@codecov

codecovBot commented Jun 4, 2019

Copy link
Copy Markdown

Codecov Report

Merging #1967 into next will decrease coverage by 0.11%.
The diff coverage is 87.5%.

Impacted file tree graph

@@ Coverage Diff @@## next #1967 +/- ##
==========================================
- Coverage 92.47% 92.35% -0.12% 
==========================================
Files 28 28 Lines 1076 1060 -16 Branches 322 322 ==========================================
- Hits 995 979 -16 
Misses 77 77 Partials 4 4
Impacted FilesCoverage Δ
client-src/default/utils/createSocketUrl.js100% <ø> (ø)⬆️
client-src/default/utils/reloadApp.js100% <ø> (ø)⬆️
client-src/default/utils/getCurrentScriptSource.js100% <ø> (ø)⬆️
client-src/default/utils/log.js100% <100%> (ø)⬆️
client-src/default/utils/sendMessage.js83.33% <100%> (-2.39%)⬇️
client-src/default/index.js91.56% <80%> (-0.66%)⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 262cfe9...a9d75d3. Read the comment docs.

@hiroppy

Copy link
Copy Markdown
ContributorAuthor

I think exports["default"] = _default will break userland.

CJS

// tsimport*assocketfrom'webpack-dev-server/client/socket'

ESM

depends on settings of tsconfig

// tsimportsocketfrom'webpack-dev-server/client/socket'

So, we will either change to the corresponding both style or change to the next branch. Anyway, I attached the label to block merging.

@hiroppy
hiroppy changed the base branch from master to nextJune 5, 2019 17:11
@hiroppy

Copy link
Copy Markdown
ContributorAuthor

@evilebottnawi please approve if possible

@hiroppy
hiroppy merged commit b197ae0 into nextJun 5, 2019
@hiroppy
hiroppy deleted the feature/use-esm branch June 5, 2019 19:08
knagaitsev pushed a commit to knagaitsev/webpack-dev-server that referenced this pull request Jul 31, 2019
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@hiroppy@alexander-akait@wizardofhogwarts