Make sure the absolute path isn't contained in the cache #5900

mischnic · 2021-02-20T21:28:53Z

Depends on #5802

Closes T-870

This should also always use / as path separators

Grepping the absolute path in .parcel-cache didn't yield any results after this change (for a simple test case).

I wasn't yet able to figure out why the the remaining few tests fail. Something with cache invalidation after file changes is wrong.
Move the new utils into utils.js in @parcel/core, no reason to put them into @parcel/utils if they're not used outside of core.
locs
still use micromatch 4?
throw an error if namers return an absolute path
Windows CI fails?

height · 2021-02-20T21:28:55Z

This pull request has been linked to and will mark 1 task as "Done" when merged:

~~T-870 Ensure all paths are relative and use platform agnostic separators in graphs and cache keys~~ (unlink task)

💡Tip: You can link multiple Height tasks to a pull request.

parcel-benchmark · 2021-02-20T21:52:28Z

Benchmark Results

Kitchen Sink 🚨

Timings

Description	Time	Difference
Cold	FAILED	-0.00ms
Cached	FAILED	-0.00ms

Cold Bundles

No bundles found, this is probably a failed build...

Cached Bundles

No bundles found, this is probably a failed build...

React HackerNews ✅

Timings

Description	Time	Difference
Cold	8.99s	-202.00ms
Cached	447.00ms	+20.00ms

Cold Bundles

Bundle	Size	Difference	Time	Difference
dist/logo.1e014c76.png	274.00b	+0.00b	203.00ms	-63.00ms 🚀

Cached Bundles

Bundle	Size	Difference	Time	Difference
dist/logo.1e014c76.png	274.00b	+0.00b	225.00ms	-75.00ms 🚀

AtlasKit Editor 🚨

Timings

Description	Time	Difference
Cold	FAILED	-0.00ms
Cached	FAILED	-0.00ms

Cold Bundles

No bundles found, this is probably a failed build...

Cached Bundles

No bundles found, this is probably a failed build...

Three.js ✅

Timings

Description	Time	Difference
Cold	6.74s	-151.00ms
Cached	431.00ms	+39.00ms ⚠️

Cold Bundles

No bundle changes detected.

Cached Bundles

No bundle changes detected.

Click here to view a detailed benchmark overview.

packages/core/core/test/TargetRequest.test.js

packages/core/core/src/types.js

packages/core/core/src/projectPath.js

devongovett · 2021-03-01T15:09:33Z

packages/core/core/src/RequestTracker.js

          }
        }

        for (let id of this.globNodeIds) {
          let globNode = this.getNode(id);
          invariant(globNode && globNode.type === 'glob');

-          if (isGlobMatch(filePath, globNode.value)) {
+          if (isGlobMatch(filePath, fromProjectPathRelative(globNode.value))) {


So filePath here is absolute, but the glob is relative? Won't that not match except if the glob starts with **/? I was thinking about how to do this for the glob resolver. Should the glob in the invalidation be relative to the project root? If so, then I think we'd need to make it absolute here.

filePath is also relative here, further up there's

let filePath = fromProjectPathRelative(_filePath);

But I ran into #5483 (comment) again, so I guess I'll have to use the absolute paths anyway (to make micromatch happy).

As a workaround, I've downgraded to micromatch 3 for now 😬

mischnic · 2021-03-04T22:25:24Z

packages/core/core/src/projectPath.js

+) => ProjectPath) &
+  ((projectRoot: FilePath, p: FilePath | void) => ProjectPath | void) &
+  // $FlowFixMe Not sure how to type properly
+  ((projectRoot: FilePath, p: ?FilePath) => ?ProjectPath) = toProjectPath_;


🤷‍♂️

elliottkember · 2021-04-13T06:31:24Z

Hi! Great work here. this PR would be a really big deal for CI / deployment environments where the build directory changes between builds. Several orders of magnitude difference. At the moment it seems the Parcel cache uses the absolute directory.

mischnic · 2021-04-24T10:36:29Z

packages/core/types/index.js

@@ -445,10 +445,9 @@ export type DependencyOptions = {|
  +meta?: Meta,
  +pipeline?: string,
  +resolveFrom?: FilePath,
-  +target?: Target,


This was apparently neither used in the plugins nor inside addDependency

devongovett · 2021-07-06T03:50:53Z

I think this is ready. I added a util that verifies that there are no references to the project root inside the cache folder after each cache test. After upgrading the source maps package to the latest release, all the tests pass.

wbinnssmith

It's a lot, but from what I can tell LGTM. cc @mischnic to review changes from @devongovett

wbinnssmith · 2021-07-07T00:36:57Z

packages/core/integration-tests/test/cache.js

  let b = await runBundle(entries, options);

+  await assertNoFilePathInCache(


* v2: (34 commits) Wrap assets recursively when any incoming dependency is wrapped (#6572) Improvements for library targets (#6570) Diagnostic for undeclared external dependencies in library builds (#6564) More bugs (#6567) Don't require `url:` for image transformer (#6565) Remove 'Name already registered with serializer' error (#6566) Fix live bindings and `this` of external CommonJS modules (#6548) JS runtime improvements (#6531) Make sure the absolute path isn't contained in the cache (#5900) Adds '@parcel/diagnostic' to dependencies (#6563) Disable workers with string literals and improve diagnostics (#6536) Bug fixes (#6541) Don't attempt to resolve URLs starting with '#' (#6504) Correctly set worker's output format if not support by environment (#6534) Babel: Recognize peerDependencies in isJSX (#6497) fix setHeaders ordering on dev server (#6500) Graph: Remove Node interface (#6530) Fix TS build script for old Node versions (#6526) Improve library targets (#6517) Fix TypeScript and other sourcemaps by always creating an initial sourcemap (#6472) ...

Seems micromatch 3 and 4 have undocumented behavior differences, So we ignore the version mismatches for now, under the assumption that the specified versions are exhibiting the expected behavior for the packages they are used in. See #5483 (comment) and #5900 (comment)

mischnic commented Feb 27, 2021

View reviewed changes

packages/core/core/test/TargetRequest.test.js Outdated Show resolved Hide resolved

mischnic commented Feb 27, 2021

View reviewed changes

packages/core/core/src/types.js Outdated Show resolved Hide resolved

mischnic commented Feb 27, 2021

View reviewed changes

packages/core/core/src/projectPath.js Outdated Show resolved Hide resolved

devongovett reviewed Mar 1, 2021

View reviewed changes

Base automatically changed from dev-dep-invalidation to v2 March 1, 2021 16:15

mischnic force-pushed the project-path branch 2 times, most recently from c3ba28f to c2dc0c0 Compare March 4, 2021 15:23

mischnic commented Mar 4, 2021

View reviewed changes

mischnic force-pushed the project-path branch from b070e3f to 697880e Compare March 10, 2021 16:25

mischnic force-pushed the project-path branch from 29867ed to 3f423c8 Compare March 22, 2021 18:57

elliottkember mentioned this pull request Apr 13, 2021

Caches not reusable #6120

Closed

mischnic mentioned this pull request Apr 19, 2021

Add dependency validation script #6155

Open

1 task

mischnic force-pushed the project-path branch from 3f423c8 to bdd7ea4 Compare April 22, 2021 22:27

mischnic commented Apr 24, 2021

View reviewed changes

mischnic force-pushed the project-path branch from 633ff89 to d30875b Compare May 9, 2021 21:57

Actually clear the ParcelConfig cache

067c5cb

mischnic mentioned this pull request May 9, 2021

Actually clear the ParcelConfig cache #6252

Merged

mischnic added 8 commits May 10, 2021 01:44

Use relative paths everywhere

e2fc4c1

Don't include leading './' in ProjectPath

7c2ed11

Remove ProcessedParcelConfig.extends

c7c76e5

Fix Packagers & Optimizers matching

4d3f7bd

Use ProjectPath for the internal SourceLocation

6f77f88

Use ProjectPath in internal SourceLocation

06b6216

Update unit tests

922d8c6

Require that resovlers return absolute paths

d1f1ebc

mischnic force-pushed the project-path branch from d30875b to d1f1ebc Compare May 9, 2021 23:45

Merge branch 'v2' into project-path

1bac2f2

devongovett added 7 commits June 27, 2021 12:04

Make __filename and __dirname relative to the project root

c6d508e

Fix environment loc

b4c1dde

Store config and defaultConfig options as relative

cbfb2fb

Treat JSX source paths as relative

ec0dce9

Resolve SASS_PATH relative to project root

95d6f13

Use a dist dir inside the project root for test

fb51ee1

Fix unit tests

2f51607

devongovett mentioned this pull request Jun 28, 2021

Don't include project root in serialized buffers parcel-bundler/source-map#73

Merged

devongovett added 5 commits June 27, 2021 19:19

lint

6a764c1

Merge branch 'v2' of github.com:parcel-bundler/parcel into project-path

226c5bc

Fixup

ab19e5d

Add test util to ensure no project root is found in the cache

b1ec19b

Upgrade source maps package to get rid of project root

e1871df

devongovett marked this pull request as ready for review July 6, 2021 03:50

devongovett added 5 commits July 5, 2021 21:00

Fix unit tests

4cc373a

Skip watcher snapshots in cache

859429f

Fix tests

5ba2571

Fix sending previous invalidations to requests

4a6f68b

Add test for moving the project root

b32556c

wbinnssmith approved these changes Jul 7, 2021

View reviewed changes

packages/core/integration-tests/test/cache.js

let b = await runBundle(entries, options);

await assertNoFilePathInCache(

Copy link

Contributor

wbinnssmith Jul 7, 2021

Choose a reason for hiding this comment

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

🤩

devongovett added 5 commits July 6, 2021 19:36

Fixes

8d874eb

Fix invalidations with lazy babel imports

6bd8676

😐

a463a44

🪟

5a440b1

Fix windows relative paths across drive letters

7a2320a

devongovett merged commit 9176b98 into v2 Jul 7, 2021

devongovett deleted the project-path branch July 7, 2021 19:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make sure the absolute path isn't contained in the cache #5900

Make sure the absolute path isn't contained in the cache #5900

mischnic commented Feb 20, 2021 •

edited

Loading

height bot commented Feb 20, 2021 •

edited

Loading

parcel-benchmark commented Feb 20, 2021 •

edited

Loading

Timings

Cold Bundles

Cached Bundles

Timings

Cold Bundles

Cached Bundles

Timings

Cold Bundles

Cached Bundles

Timings

Cold Bundles

Cached Bundles

devongovett Mar 1, 2021

mischnic Mar 1, 2021

mischnic Mar 1, 2021

mischnic Mar 4, 2021

elliottkember commented Apr 13, 2021 •

edited

Loading

mischnic Apr 24, 2021

devongovett commented Jul 6, 2021

wbinnssmith left a comment

wbinnssmith Jul 7, 2021

		let b = await runBundle(entries, options);

		await assertNoFilePathInCache(

Make sure the absolute path isn't contained in the cache #5900

Make sure the absolute path isn't contained in the cache #5900

Conversation

mischnic commented Feb 20, 2021 • edited Loading

height bot commented Feb 20, 2021 • edited Loading

parcel-benchmark commented Feb 20, 2021 • edited Loading

Benchmark Results

Timings

Cold Bundles

Cached Bundles

Timings

Cold Bundles

Cached Bundles

Timings

Cold Bundles

Cached Bundles

Timings

Cold Bundles

Cached Bundles

devongovett Mar 1, 2021

Choose a reason for hiding this comment

mischnic Mar 1, 2021

Choose a reason for hiding this comment

mischnic Mar 1, 2021

Choose a reason for hiding this comment

mischnic Mar 4, 2021

Choose a reason for hiding this comment

elliottkember commented Apr 13, 2021 • edited Loading

mischnic Apr 24, 2021

Choose a reason for hiding this comment

devongovett commented Jul 6, 2021

wbinnssmith left a comment

Choose a reason for hiding this comment

wbinnssmith Jul 7, 2021

Choose a reason for hiding this comment

mischnic commented Feb 20, 2021 •

edited

Loading

height bot commented Feb 20, 2021 •

edited

Loading

parcel-benchmark commented Feb 20, 2021 •

edited

Loading

elliottkember commented Apr 13, 2021 •

edited

Loading