Skip to content

chore: replace source-map with source-map-js - #6556

Merged
patak-cat merged 1 commit into
vitejs:mainfrom
TrySound:source-map-js
Jan 18, 2022
Merged

patak-cat merged 1 commit into
vitejs:mainfrom
TrySound:source-map-js

Conversation

@TrySound

@TrySound TrySound commented Jan 18, 2022 •

Copy link
Copy Markdown
Contributor

Description

source-map-js is a js fork with applied optimisations comparable with
rust optimisation in latest source-map versions.

Staying on js allows us to bundle package and not increase install size.

As a bonus I added support for importing from lib directly so we could
avoid bundling whole package. See 7rulnik/source-map-js#13

du -ck dist/node

before: 4164 kB
after: 4140 kB

Additional context


What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines.
  • Read the Pull Request Guidelines and follow the Commit Convention.
  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
  • Ideally, include relevant tests that fail without this PR but pass with it.

@Niputi Niputi added dependencies Pull requests that update a dependency file p1-chore Doesn't change code behavior (priority) labels Jan 18, 2022
@Niputi

Niputi commented Jan 18, 2022

Copy link
Copy Markdown
Contributor

why does rollup need a bump?

@TrySound

Copy link
Copy Markdown
Contributor Author

Hm... I didn't touch rollup. Though rebased from december branch. Will try to revert.

@Niputi Niputi changed the title fix: replace source-map with source-map-js chore: replace source-map with source-map-js Jan 18, 2022
source-map-js is a js fork with applied optimisations comparable with
rust optimisation in latest source-map versions.

Staying on js allows us to bundle package and not increase install size.

As a bonus I added support for importing from lib directly so we could
avoid bundling whole package. See 7rulnik/source-map-js#13

```
du -ck dist/node
```
before: 4164 kB
after: 4140 kB
Comment thread packages/vite/LICENSE.md Resolved

@Niputi Niputi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

💚

@patak-cat
patak-cat merged commit 7b95f4d into vitejs:main Jan 18, 2022
@TrySound
TrySound deleted the source-map-js branch January 18, 2022 20:42
@patak-cat

Copy link
Copy Markdown
Member

I need GitHub to let us add the 🔥 emoji as reactions

import type { ModuleGraph } from './moduleGraph'
import { performance } from 'perf_hooks'
import { SourceMapConsumer } from 'source-map'
import { SourceMapConsumer } from 'source-map-js/lib/source-map-consumer'

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.

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 like we don't need the /lib/source-map-consumer part, as source-map-js main module exports SourceMapConsumer too.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is done on purpose to reduce bundle size.

aleclarson pushed a commit to aleclarson/vite that referenced this pull request Feb 12, 2022
source-map-js is a js fork with applied optimisations comparable with
rust optimisation in latest source-map versions.

Staying on js allows us to bundle package and not increase install size.

As a bonus I added support for importing from lib directly so we could
avoid bundling whole package. See 7rulnik/source-map-js#13

```
du -ck dist/node
```
before: 4164 kB
after: 4140 kB
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file p1-chore Doesn't change code behavior (priority)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants