-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Babel ast location, diagnostic, and source location remapping #6238
Changes from 38 commits
fc88e8b
424888a
6c1bb67
309cfaa
ef8dca4
72cc8a6
9096141
dbdd78b
1efc8d6
d624437
823da35
5eebda4
8d47c46
70c00d6
95acc95
bf6591b
64b434a
e509474
df3ed6d
7266f84
f6e0e03
f1c6d66
1cb16cd
d6da582
754ad53
4a83253
d96a906
6c8d560
842d2cf
269d2c9
f52e2bf
c39fa92
51aa11e
6aeb81a
a62fb08
a2819a3
c674b9c
337f67d
d2f4f86
789e4e1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,6 +14,7 @@ import type ParcelConfig from './ParcelConfig'; | |
import type PluginOptions from './public/PluginOptions'; | ||
import type {RunAPI} from './RequestTracker'; | ||
|
||
import path from 'path'; | ||
import assert from 'assert'; | ||
import invariant from 'assert'; | ||
import nullthrows from 'nullthrows'; | ||
|
@@ -22,7 +23,9 @@ import BundleGraph from './public/BundleGraph'; | |
import InternalBundleGraph from './BundleGraph'; | ||
import {NamedBundle} from './public/Bundle'; | ||
import {PluginLogger} from '@parcel/logger'; | ||
import {md5FromString} from '@parcel/utils'; | ||
import ThrowableDiagnostic, {errorToDiagnostic} from '@parcel/diagnostic'; | ||
import SourceMap from '@parcel/source-map'; | ||
import {dependencyToInternalDependency} from './public/Dependency'; | ||
import createAssetGraphRequest from './requests/AssetGraphRequest'; | ||
import {createDevDependency, runDevDepRequest} from './requests/DevDepRequest'; | ||
|
@@ -76,8 +79,20 @@ export default async function applyRuntimes({ | |
if (applied) { | ||
let runtimeAssets = Array.isArray(applied) ? applied : [applied]; | ||
for (let {code, dependency, filePath, isEntry} of runtimeAssets) { | ||
let sourceName = path.join( | ||
path.dirname(filePath), | ||
`runtime-${md5FromString(code)}.${bundle.type}`, | ||
); | ||
|
||
let sourcemap = SourceMap.generateEmptyMap({ | ||
projectRoot: pluginOptions.projectRoot, | ||
sourceName, | ||
sourceContent: code, | ||
}); | ||
|
||
let assetGroup = { | ||
code, | ||
mapBuffer: sourcemap.toBuffer(), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is still going to be an issue as it (along with the code) gets serialized into the bundlegraph. cc @DeMoorJasper @devongovett There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. Do we actually need the sourcemap here? It's empty and only contains the same code as is already passed through the request. If there's no existing map, won't it already be set as the source content during transformation? I guess we should have an integration test for runtimes + sourcemaps. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Another issue is that with empty maps all of the columns mappings are zero when the empty map is extended. If we overwrote the |
||
filePath, | ||
env: bundle.env, | ||
// Runtime assets should be considered source, as they should be | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
{ | ||
"presets": ["@babel/preset-react", "@babel/preset-flow"] | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
// @flow | ||
|
||
import * as React from "react"; | ||
import * as ReactDOM from "react-dom"; | ||
|
||
type Props = {| | ||
bar: string, | ||
|}; | ||
|
||
function App(props: Props) { | ||
return <div>{props.bar}</div>; | ||
} | ||
|
||
ReactDOM.render(<App bar="bar" />, document.getElementById("root")); |
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.
Why is this stored separately on the asset rather than in the source map? If there's no existing source map, should we just initialize an empty one containing only the original 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.
Because of this line: https://github.com/parcel-bundler/parcel/blob/v2/packages/core/core/src/UncommittedAsset.js#L102