Skip to content
This repository has been archived by the owner on May 22, 2024. It is now read-only.

feat: add traceWithNft feature flag #769

Merged
merged 5 commits into from
Oct 25, 2021
Merged

Conversation

eduardoboucas
Copy link
Member

@eduardoboucas eduardoboucas commented Oct 25, 2021

- Summary

Adds a traceWithNft feature flag which, when active, will trace dependencies with NFT by default.

It also adds aws-sdk to NFT's ignore property, in line with what we're doing for ZISI and esbuild.

- Test plan

Adjusted existing tests.

- A picture of a cute animal (not mandatory but encouraged)

sddefault

@eduardoboucas eduardoboucas added the type: feature code contributing to the implementation of a feature and/or user facing functionality label Oct 25, 2021
@eduardoboucas eduardoboucas merged commit 0ef6b37 into main Oct 25, 2021
@eduardoboucas eduardoboucas deleted the feat/tracewithnft-flag branch October 25, 2021 13:17
@@ -70,6 +82,7 @@ const traceFilesAndTranspile = async function ({
const { fileList: dependencyPaths } = await nodeFileTrace([mainFile], {
base: basePath,
cache,
ignore: ignoreFunction,
Copy link
Member

@Skn0tt Skn0tt Jun 27, 2023

Choose a reason for hiding this comment

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

Just stumbled across this a millennia later. I see we're passing a custom ignoreFunction here that ends up doing glob matching. NFT docs say that you can pass an array of globs here, instead: https://github.com/vercel/nft/blob/314fd539335002273e77ba20d9832042af2a0bab/readme.md?plain=1#L166

What was the reasons against doing that? Is there a perf improvement / optimisation we could get from using an array here?

Copy link
Member Author

Choose a reason for hiding this comment

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

No idea, to be honest. Maybe it had to do with cross-OS path normalisation (since I see unixify in there)? Or maybe the array option wasn't available at the time (or I simply didn't realise it was)?

Copy link
Member

Choose a reason for hiding this comment

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

It was available at the time - introduced in a commit named "first commit" :D vercel/nft@31d836e

I'll try replacing this function with an array in a different PR. If that doesn't break our tests (i'll check if we have one) then let's use the array instead.

@Skn0tt Skn0tt mentioned this pull request Jun 27, 2023
5 tasks
Skn0tt pushed a commit to netlify/build that referenced this pull request May 21, 2024
* feat: add aws-sdk to NFT ignore

* feat: add traceWithNft flag

* fix: make path Windows-friendly

* fix: make ignore function compatible with Windows paths

* refactor: adjust ignore glob
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type: feature code contributing to the implementation of a feature and/or user facing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants