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.

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

Diagnostic for undeclared external dependencies in library builds #6564

Merged
merged 6 commits into from Jul 9, 2021

Conversation

devongovett
Copy link
Member

@devongovett devongovett commented Jul 7, 2021

Fixes T-1065. Fixes T-1076.

This ensures that external dependencies in libraries (i.e. excluded due to includeNodeModules) are defined in package.json. This helps with both explicit dependencies, as well as implicit dependencies like @swc/helpers.

In addition, in non-library mode, we ensure that even if includeNodeModules is false, helpers are always bundled.


@height
Copy link

height bot commented Jul 7, 2021

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

  • T-1076 bundle @swc/helpers in non-library mode even with includeNodeModules: false (unlink task)
  • T-1065 import {extends as $3whmL$extends} from "@swc/helpers"; in library mode (unlink task)

@mischnic
Copy link
Member

mischnic commented Jul 7, 2021

I can see why you used sourcePath because of the swc helpers, but shouldn't it conceptually check relative to resolveFrom?

@parcel-benchmark
Copy link

parcel-benchmark commented Jul 7, 2021

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.13s -92.00ms
Cached 419.00ms +7.00ms

Cold Bundles

No bundle changes detected.

Cached Bundles

Bundle Size Difference Time Difference
dist/logo.1e014c76.png 274.00b +0.00b 208.00ms +19.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 5.90s -55.00ms
Cached 361.00ms +17.00ms

Cold Bundles

No bundle changes detected.

Cached Bundles

No bundle changes detected.

Click here to view a detailed benchmark overview.

@devongovett
Copy link
Member Author

sourcePath is the file the dependency was defined in, so I think it makes sense to use that for diagnostics. resolveFrom is probably more internal.

@devongovett devongovett merged commit 278cfe8 into v2 Jul 9, 2021
@devongovett devongovett deleted the external-deps branch July 9, 2021 02:14
lettertwo added a commit that referenced this pull request Jul 13, 2021
* 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)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants