From 6be63be102e697dc68bb0c20be8dda47969ef726 Mon Sep 17 00:00:00 2001 From: Devon Govett Date: Wed, 7 Jul 2021 23:18:44 -0700 Subject: [PATCH 1/5] Reload the page on HMR when not all changed assets are handled --- packages/runtimes/hmr/src/loaders/hmr-runtime.js | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/packages/runtimes/hmr/src/loaders/hmr-runtime.js b/packages/runtimes/hmr/src/loaders/hmr-runtime.js index bb4a01b42a9..3e07d8c0bd9 100644 --- a/packages/runtimes/hmr/src/loaders/hmr-runtime.js +++ b/packages/runtimes/hmr/src/loaders/hmr-runtime.js @@ -100,15 +100,12 @@ if ((!parent || !parent.isParcelRequire) && typeof WebSocket !== 'undefined') { let assets = data.assets.filter(asset => asset.envHash === HMR_ENV_HASH); // Handle HMR Update - var handled = false; - assets.forEach(asset => { - var didAccept = + let handled = assets.every(asset => { + return ( asset.type === 'css' || (asset.type === 'js' && - hmrAcceptCheck(module.bundle.root, asset.id, asset.depsByBundle)); - if (didAccept) { - handled = true; - } + hmrAcceptCheck(module.bundle.root, asset.id, asset.depsByBundle)) + ); }); if (handled) { From a752cb7d44a957eba8c7f4ea147f27b7f9c0abb0 Mon Sep 17 00:00:00 2001 From: Devon Govett Date: Wed, 7 Jul 2021 23:27:35 -0700 Subject: [PATCH 2/5] Fix React Fast Refresh with automatic JSX runtime --- .../react-refresh-wrap/src/ReactRefreshWrapTransformer.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/transformers/react-refresh-wrap/src/ReactRefreshWrapTransformer.js b/packages/transformers/react-refresh-wrap/src/ReactRefreshWrapTransformer.js index fe0cd09948f..81cf747b909 100644 --- a/packages/transformers/react-refresh-wrap/src/ReactRefreshWrapTransformer.js +++ b/packages/transformers/react-refresh-wrap/src/ReactRefreshWrapTransformer.js @@ -14,7 +14,9 @@ function shouldExclude(asset, options) { asset.env.isWorker() || asset.env.isWorklet() || options.mode !== 'development' || - !asset.getDependencies().find(v => v.specifier === 'react') + !asset + .getDependencies() + .find(v => v.specifier === 'react' || v.specifier === 'react/jsx-runtime') ); } From 6eeaa02c1270c92ea386fd7e3caae4cf10dbee04 Mon Sep 17 00:00:00 2001 From: Devon Govett Date: Thu, 8 Jul 2021 00:10:09 -0700 Subject: [PATCH 3/5] Rebuild when a failed resolution is fixed --- packages/core/core/src/Transformation.js | 26 ++-- .../core/core/src/requests/PathRequest.js | 122 ++++++++++++------ .../core/integration-tests/test/watcher.js | 26 ++++ .../node-resolver-core/src/NodeResolver.js | 2 + 4 files changed, 120 insertions(+), 56 deletions(-) diff --git a/packages/core/core/src/Transformation.js b/packages/core/core/src/Transformation.js index ecac842a674..5f18055e2c8 100644 --- a/packages/core/core/src/Transformation.js +++ b/packages/core/core/src/Transformation.js @@ -660,19 +660,13 @@ export default class Transformation { const logger = new PluginLogger({origin: transformerName}); const resolve = async (from: FilePath, to: string): Promise => { - let result: {| - assetGroup: AssetGroup, - invalidateOnFileCreate?: Array, - invalidateOnFileChange?: Array, - |} = nullthrows( - await pipeline.resolverRunner.resolve( - createDependency(this.options.projectRoot, { - env: asset.value.env, - specifier: to, - specifierType: 'esm', // ??? - sourcePath: from, - }), - ), + let result = await pipeline.resolverRunner.resolve( + createDependency(this.options.projectRoot, { + env: asset.value.env, + specifier: to, + specifierType: 'esm', // ??? + sourcePath: from, + }), ); if (result.invalidateOnFileCreate) { @@ -694,9 +688,13 @@ export default class Transformation { } } + if (result.diagnostics && result.diagnostics.length > 0) { + throw new ThrowableDiagnostic({diagnostic: result.diagnostics}); + } + return fromProjectPath( this.options.projectRoot, - result.assetGroup.filePath, + nullthrows(result.assetGroup).filePath, ); }; diff --git a/packages/core/core/src/requests/PathRequest.js b/packages/core/core/src/requests/PathRequest.js index c90b07103ac..d3c18126af9 100644 --- a/packages/core/core/src/requests/PathRequest.js +++ b/packages/core/core/src/requests/PathRequest.js @@ -5,6 +5,7 @@ import type { FileCreateInvalidation, FilePath, QueryParameters, + ResolveResult, } from '@parcel/types'; import type {StaticRunOpts} from '../RequestTracker'; import type {AssetGroup, Dependency, ParcelOptions} from '../types'; @@ -72,28 +73,35 @@ async function run({input, api, options}: RunOpts) { options, config, }); - let result: ?ResolverResult = await resolverRunner.resolve(input.dependency); - - if (result != null) { - if (result.invalidateOnFileCreate) { - for (let file of result.invalidateOnFileCreate) { - api.invalidateOnFileCreate( - invalidateOnFileCreateToInternal(options.projectRoot, file), - ); - } + let result: ResolverResult = await resolverRunner.resolve(input.dependency); + + if (result.invalidateOnFileCreate) { + for (let file of result.invalidateOnFileCreate) { + api.invalidateOnFileCreate( + invalidateOnFileCreateToInternal(options.projectRoot, file), + ); } + } - if (result.invalidateOnFileChange) { - for (let filePath of result.invalidateOnFileChange) { - let pp = toProjectPath(options.projectRoot, filePath); - api.invalidateOnFileUpdate(pp); - api.invalidateOnFileDelete(pp); - } + if (result.invalidateOnFileChange) { + for (let filePath of result.invalidateOnFileChange) { + let pp = toProjectPath(options.projectRoot, filePath); + api.invalidateOnFileUpdate(pp); + api.invalidateOnFileDelete(pp); } + } + if (result.assetGroup) { api.invalidateOnFileDelete(result.assetGroup.filePath); return result.assetGroup; } + + if (result.diagnostics && result.diagnostics.length > 0) { + let err = new ThrowableDiagnostic({diagnostic: result.diagnostics}); + // $FlowFixMe[prop-missing] + err.code = 'MODULE_NOT_FOUND'; + throw err; + } } type ResolverRunnerOpts = {| @@ -102,9 +110,10 @@ type ResolverRunnerOpts = {| |}; type ResolverResult = {| - assetGroup: AssetGroup, + assetGroup: ?AssetGroup, invalidateOnFileCreate?: Array, invalidateOnFileChange?: Array, + diagnostics?: Array, |}; export class ResolverRunner { @@ -118,10 +127,10 @@ export class ResolverRunner { this.pluginOptions = new PluginOptions(this.options); } - async getThrowableDiagnostic( + async getDiagnostic( dependency: Dependency, message: string, - ): Async { + ): Async { let diagnostic: Diagnostic = { message, origin: '@parcel/core', @@ -143,10 +152,10 @@ export class ResolverRunner { ]; } - return new ThrowableDiagnostic({diagnostic}); + return diagnostic; } - async resolve(dependency: Dependency): Promise { + async resolve(dependency: Dependency): Promise { let dep = new PublicDependency(dependency, this.options); report({ type: 'buildProgress', @@ -174,12 +183,17 @@ export class ResolverRunner { if (dep.specifierType === 'url') { // This may be a url protocol or scheme rather than a pipeline, such as // `url('http://example.com/foo.png')` - return null; + return {assetGroup: null}; } else { - throw await this.getThrowableDiagnostic( - dependency, - md`Unknown pipeline: ${pipeline}.`, - ); + return { + assetGroup: null, + diagnostics: [ + await this.getDiagnostic( + dependency, + md`Unknown pipeline: ${pipeline}.`, + ), + ], + }; } } } @@ -187,11 +201,11 @@ export class ResolverRunner { if (dep.specifierType === 'url') { if (dependency.specifier.startsWith('//')) { // A protocol-relative URL, e.g `url('//example.com/foo.png')` - return null; + return {assetGroup: null}; } if (dependency.specifier.startsWith('#')) { // An ID-only URL, e.g. `url(#clip-path)` for CSS rules - return null; + return {assetGroup: null}; } } filePath = dependency.specifier; @@ -201,10 +215,15 @@ export class ResolverRunner { if (dep.specifierType === 'url') { let parsed = URL.parse(filePath); if (typeof parsed.pathname !== 'string') { - throw await this.getThrowableDiagnostic( - dependency, - md`Received URL without a pathname ${filePath}.`, - ); + return { + assetGroup: null, + diagnostics: [ + await this.getDiagnostic( + dependency, + md`Received URL without a pathname ${filePath}.`, + ), + ], + }; } filePath = decodeURIComponent(parsed.pathname); if (parsed.query != null) { @@ -226,6 +245,8 @@ export class ResolverRunner { filePath = path.join(this.options.projectRoot, filePath); } let diagnostics: Array = []; + let invalidateOnFileCreate = []; + let invalidateOnFileChange = []; for (let resolver of resolvers) { try { let result = await resolver.plugin.resolve({ @@ -249,8 +270,20 @@ export class ResolverRunner { dependency.priority = Priority[result.priority]; } + if (result.invalidateOnFileCreate) { + invalidateOnFileCreate.push(...result.invalidateOnFileCreate); + } + + if (result.invalidateOnFileChange) { + invalidateOnFileChange.push(...result.invalidateOnFileChange); + } + if (result.isExcluded) { - return null; + return { + assetGroup: null, + invalidateOnFileCreate, + invalidateOnFileChange, + }; } if (result.filePath != null) { @@ -278,8 +311,8 @@ export class ResolverRunner { : result.pipeline, isURL: dep.specifierType === 'url', }, - invalidateOnFileCreate: result.invalidateOnFileCreate, - invalidateOnFileChange: result.invalidateOnFileChange, + invalidateOnFileCreate, + invalidateOnFileChange, }; } @@ -311,7 +344,11 @@ export class ResolverRunner { } if (dep.isOptional) { - return null; + return { + assetGroup: null, + invalidateOnFileCreate, + invalidateOnFileChange, + }; } let resolveFrom = dependency.resolveFrom ?? dependency.sourcePath; @@ -320,19 +357,20 @@ export class ResolverRunner { ? normalizePath(fromProjectPathRelative(resolveFrom)) : ''; - // $FlowFixMe because of the err.code assignment - let err = await this.getThrowableDiagnostic( + let diagnostic = await this.getDiagnostic( dependency, md`Failed to resolve '${dependency.specifier}' ${ dir ? `from '${dir}'` : '' }`, ); - // Merge diagnostics - err.diagnostics.push(...diagnostics); - // $FlowFixMe[prop-missing] - err.code = 'MODULE_NOT_FOUND'; + diagnostics.unshift(diagnostic); - throw err; + return { + assetGroup: null, + invalidateOnFileCreate, + invalidateOnFileChange, + diagnostics, + }; } } diff --git a/packages/core/integration-tests/test/watcher.js b/packages/core/integration-tests/test/watcher.js index 7cb005b7d14..a0675d4629b 100644 --- a/packages/core/integration-tests/test/watcher.js +++ b/packages/core/integration-tests/test/watcher.js @@ -445,4 +445,30 @@ describe('watcher', function() { }, ]); }); + + it('should rebuild if a missing file is added', async function() { + await outputFS.mkdirp(inputDir); + await outputFS.writeFile( + path.join(inputDir, '/index.js'), + 'import {other} from "./other";\nexport default other;', + 'utf8', + ); + + let b = bundler(path.join(inputDir, 'index.js'), {inputFS: overlayFS}); + subscription = await b.watch(); + let buildEvent = await getNextBuild(b); + assert.equal(buildEvent.type, 'buildFailure'); + + await outputFS.writeFile( + path.join(inputDir, '/other.js'), + 'export const other = 2;', + 'utf8', + ); + + buildEvent = await getNextBuild(b); + assert.equal(buildEvent.type, 'buildSuccess'); + + let res = await run(buildEvent.bundleGraph); + assert.equal(res.default, 2); + }); }); diff --git a/packages/utils/node-resolver-core/src/NodeResolver.js b/packages/utils/node-resolver-core/src/NodeResolver.js index aced4e0913f..71ceac305d0 100644 --- a/packages/utils/node-resolver-core/src/NodeResolver.js +++ b/packages/utils/node-resolver-core/src/NodeResolver.js @@ -186,6 +186,8 @@ export default class NodeResolver { if (err instanceof ThrowableDiagnostic) { return { diagnostics: err.diagnostics, + invalidateOnFileCreate: ctx.invalidateOnFileCreate, + invalidateOnFileChange: [...ctx.invalidateOnFileChange], }; } else { throw err; From 70b679b28849a6acdd9cfbbf0ca2c73700dfc2e3 Mon Sep 17 00:00:00 2001 From: Devon Govett Date: Thu, 8 Jul 2021 00:13:28 -0700 Subject: [PATCH 4/5] Constructing a new URL should return an object, not a string --- .../core/integration-tests/test/javascript.js | 4 ++-- .../js/core/src/dependency_collector.rs | 18 +++++++++++++++++- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/packages/core/integration-tests/test/javascript.js b/packages/core/integration-tests/test/javascript.js index e7afd4d6789..50050ebcd8b 100644 --- a/packages/core/integration-tests/test/javascript.js +++ b/packages/core/integration-tests/test/javascript.js @@ -2037,7 +2037,7 @@ describe('javascript', function() { let output = await run(b); assert(/^http:\/\/localhost\/test\.[0-9a-f]+\.txt$/.test(output.default)); let stats = await outputFS.stat( - path.join(distDir, url.parse(output.default).pathname), + path.join(distDir, output.default.pathname), ); assert.equal(stats.size, 9); }); @@ -2064,7 +2064,7 @@ describe('javascript', function() { let output = await run(b); assert(/^http:\/\/localhost\/test\.[0-9a-f]+\.txt$/.test(output.default)); let stats = await outputFS.stat( - path.join(distDir, url.parse(output.default).pathname), + path.join(distDir, output.default.pathname), ); assert.equal(stats.size, 9); }); diff --git a/packages/transformers/js/core/src/dependency_collector.rs b/packages/transformers/js/core/src/dependency_collector.rs index 4b6ab176e8a..50e1db8b085 100644 --- a/packages/transformers/js/core/src/dependency_collector.rs +++ b/packages/transformers/js/core/src/dependency_collector.rs @@ -705,12 +705,28 @@ impl<'a> Fold for DependencyCollector<'a> { use ast::*; if let Some((specifier, span)) = self.match_import_meta_url(&node, self.decls) { - return self.add_url_dependency( + let url = self.add_url_dependency( specifier.clone(), span, DependencyKind::URL, self.config.source_type, ); + + // If this is a library, we will already have a URL object. Otherwise, we need to + // construct one from the string returned by the JSRuntime. + if !self.config.is_library { + return Expr::New(NewExpr { + span: DUMMY_SP, + callee: Box::new(Expr::Ident(Ident::new(js_word!("URL"), DUMMY_SP))), + args: Some(vec![ExprOrSpread { + expr: Box::new(url), + spread: None, + }]), + type_args: None, + }); + } + + return url; } let is_require = match &node { From 9675955b8f5b101e96e0a238e4f91d21e9e8cced Mon Sep 17 00:00:00 2001 From: Devon Govett Date: Thu, 8 Jul 2021 00:14:38 -0700 Subject: [PATCH 5/5] lint --- packages/core/core/src/Transformation.js | 2 -- packages/core/core/src/requests/PathRequest.js | 1 - 2 files changed, 3 deletions(-) diff --git a/packages/core/core/src/Transformation.js b/packages/core/core/src/Transformation.js index 5f18055e2c8..63f3b4732bd 100644 --- a/packages/core/core/src/Transformation.js +++ b/packages/core/core/src/Transformation.js @@ -2,7 +2,6 @@ import type { FilePath, - FileCreateInvalidation, GenerateOutput, Transformer, TransformerResult, @@ -12,7 +11,6 @@ import type { import type {WorkerApi} from '@parcel/workers'; import type { Asset as AssetValue, - AssetGroup, TransformationRequest, RequestInvalidation, Config, diff --git a/packages/core/core/src/requests/PathRequest.js b/packages/core/core/src/requests/PathRequest.js index d3c18126af9..9ec81c8bb3a 100644 --- a/packages/core/core/src/requests/PathRequest.js +++ b/packages/core/core/src/requests/PathRequest.js @@ -5,7 +5,6 @@ import type { FileCreateInvalidation, FilePath, QueryParameters, - ResolveResult, } from '@parcel/types'; import type {StaticRunOpts} from '../RequestTracker'; import type {AssetGroup, Dependency, ParcelOptions} from '../types';