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

fix: update sourcemap in importAnalysisBuild #7825

Merged
merged 4 commits into from Apr 21, 2022
Merged

fix: update sourcemap in importAnalysisBuild #7825

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9cd2a31
fix: update sourcemap in importAnalysisBuild
sapphi-red Apr 20, 2022
79c3418
perf: use single magic-string instance
sapphi-red Apr 21, 2022
319dcad
refactor: use preloadMarkerWithQuote
sapphi-red Apr 21, 2022
3f6d6e6
fix: when preload marker missing
sapphi-red Apr 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
32 changes: 27 additions & 5 deletions packages/vite/src/node/plugins/importAnalysisBuild.ts
View file
Open in desktop
Expand Up @@ -4,10 +4,12 @@ import type { Plugin } from '../plugin'
import MagicString from 'magic-string'
import type { ImportSpecifier } from 'es-module-lexer'
import { init, parse as parseImports } from 'es-module-lexer'
import type { OutputChunk } from 'rollup'
import type { OutputChunk, SourceMap } from 'rollup'
import { isCSSRequest, removedPureCssFilesCache } from './css'
import { transformImportGlob } from '../importGlob'
import { bareImportRE } from '../utils'
import { bareImportRE, combineSourcemaps } from '../utils'
import type { RawSourceMap } from '@ampproject/remapping'
import { genSourceMapUrl } from '../server/sourcemap'

/**
* A flag for injected helpers. This flag will be set to `false` if the output
Expand Down Expand Up @@ -263,6 +265,25 @@ export function buildImportAnalysisPlugin(config: ResolvedConfig): Plugin {
this.error(e, e.idx)
}

const updateChunkCode = (ms: MagicString) => {
if (!ms.hasChanged()) return

chunk.code = ms.toString()
if (config.build.sourcemap && chunk.map) {
const nextMap = ms.generateMap({
source: chunk.fileName,
hires: true
})
const map = combineSourcemaps(
chunk.fileName,
[nextMap as RawSourceMap, chunk.map as RawSourceMap],
false
) as SourceMap
map.toUrl = () => genSourceMapUrl(map)
chunk.map = map
}
}

if (imports.length) {
const s = new MagicString(code)
for (let index = 0; index < imports.length; index++) {
Expand Down Expand Up @@ -345,13 +366,14 @@ export function buildImportAnalysisPlugin(config: ResolvedConfig): Plugin {
)
}
}
chunk.code = s.toString()
// TODO source map
updateChunkCode(s)
}

// there may still be markers due to inlined dynamic imports, remove
// all the markers regardless
chunk.code = chunk.code.replace(preloadMarkerRE, 'void 0')
const s = new MagicString(chunk.code)
Copy link
Member

Choose a reason for hiding this comment

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

Could we create this MagicString at the top so we only use one, and only need to combine the source maps once?

Copy link
Member Author

Choose a reason for hiding this comment

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

It needs to replace markers which were not replaced. Using the same MagicString instance was doing a different behavior here.

import MagicString from 'magic-string';

const mark = '__VITE_PRELOAD__';

const code = `
  import('foo.js', "__VITE_PRELOAD__")
  "__VITE_PRELOAD__"
`;

const s = new MagicString(code);

s.overwrite(
  code.indexOf(mark) - 1,
  code.indexOf(mark) + mark.length + 1,
  '["deps.js"]',
  {
    contentOnly: true,
  }
);

/*
  import('foo.js', ["deps.js"])
  "__VITE_PRELOAD__"
*/
console.log(s.toString());

s.replace(new RegExp(`"${mark}"`, 'g'), 'void 0');

/*
  import('foo.js', void 0)
  void 0
*/
console.log(s.toString());

To change this, it will need to store positions at line 355 and then do the replace without s.replace at line 375 with skipping the stored positions.
Should I do so? (It's going to be easier than I first thought.)

Copy link
Member

Choose a reason for hiding this comment

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

To change this, it will need to store positions at line 355 and then do the replace without s.replace at line 375 with skipping the stored positions.
Should I do so? (It's going to be easier than I first thought.)

Sounds like a good thing to do now that I re-look the code, should improve perf too.

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 have changed it 👍

s.replace(preloadMarkerRE, 'void 0')
updateChunkCode(s)
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion packages/vite/src/node/server/sourcemap.ts
View file
Open in desktop
Expand Up @@ -61,7 +61,7 @@ export async function injectSourcesContent(
}
}

function genSourceMapUrl(map: SourceMap | string | undefined) {
export function genSourceMapUrl(map: SourceMap | string | undefined) {
if (typeof map !== 'string') {
map = JSON.stringify(map)
}
Expand Down
7 changes: 4 additions & 3 deletions packages/vite/src/node/utils.ts
View file
Open in desktop
Expand Up @@ -605,7 +605,8 @@ const nullSourceMap: RawSourceMap = {
}
export function combineSourcemaps(
filename: string,
sourcemapList: Array<DecodedSourceMap | RawSourceMap>
sourcemapList: Array<DecodedSourceMap | RawSourceMap>,
excludeContent = true
): RawSourceMap {
if (
sourcemapList.length === 0 ||
Expand Down Expand Up @@ -635,7 +636,7 @@ export function combineSourcemaps(
const useArrayInterface =
sourcemapList.slice(0, -1).find((m) => m.sources.length !== 1) === undefined
if (useArrayInterface) {
map = remapping(sourcemapList, () => null, true)
map = remapping(sourcemapList, () => null, excludeContent)
} else {
map = remapping(
sourcemapList[0],
Expand All @@ -646,7 +647,7 @@ export function combineSourcemaps(
return null
}
},
true
excludeContent
)
}
if (!map.file) {
Expand Down