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.
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
fix: update sourcemap in importAnalysisBuild #7825
fix: update sourcemap in importAnalysisBuild #7825
Conversation
} | ||
|
||
// 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) |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.)
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have changed it 👍
Co-authored-by: patak <matias.capeletto@gmail.com>
Description
importAnalysisBuild was breaking sourcemaps when there is a dynamic import.
This is a reproduction.
https://stackblitz.com/edit/vitejs-vite-ipkqha?file=main.js&terminal=dev
2
has changed from line 4 to 5)Additional context
What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123
).