Skip to content
This repository was archived by the owner on Apr 6, 2023. It is now read-only.

Conversation

@danielroe
Copy link
Member

πŸ”— Linked issue

resolves nuxt/nuxt#14136

❓ Type of change

  • πŸ“– Documentation (updates to the documentation or readme)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • πŸ‘Œ Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

This fixes a regression introduced by #5042, by:

  1. only adding layers within node_modules to autoImports include.
  2. not force-transforming all files in include but only the ones that match the test for file time

πŸ“ Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

@danielroe danielroe added bug Something isn't working πŸ”₯ p5-urgent Priority 5: build-breaking bugs that affect most users and should be fixed ASAP labels Jun 11, 2022
@danielroe danielroe requested a review from pi0 June 11, 2022 20:44
@danielroe danielroe self-assigned this Jun 11, 2022
@netlify
Copy link

netlify bot commented Jun 11, 2022

βœ… Deploy Preview for nuxt3-docs canceled.

Name Link
πŸ”¨ Latest commit 4983968
πŸ” Latest deploy log https://app.netlify.com/sites/nuxt3-docs/deploys/62a4fe96dbea3e0009ebac0d

const include = options.transform?.include || []

// Custom includes
if (include.some(pattern => id.match(pattern))) {
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we keep this check after exclude? (ie: explicit include without explicit exclude). It is current behavior before (https://github.com/nuxt/framework/pull/5042/files#diff-1edf1ef633d6d36d0410fe36249ca18b1f5fd1a58b658296696698c2fc542ee9L18)

Copy link
Member Author

@danielroe danielroe Jun 12, 2022

Choose a reason for hiding this comment

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

No, because we exclude node_modules, and therefore we would never be able to override to include layers within node_modules.

Copy link
Member

@pi0 pi0 Jun 12, 2022

Choose a reason for hiding this comment

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

Then maybe reverse?

  • Include checks:
    • include node_modules/<layer>/<!node_modules>
  • Exclude checks: (if not an include pattern match)
    • exclude node_modules

Combined method with && in this PR works but makes it unable to explicitly include a pattern.

Copy link
Member Author

@danielroe danielroe Jun 12, 2022

Choose a reason for hiding this comment

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

The point is that we still need to perform checks on the file extension.

I don't know what you mean by:

Combined method with && in this PR works but makes it unable to explicitly include a pattern.

Copy link
Member

Choose a reason for hiding this comment

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

Previously we had two checks for exlude list and include list option. We are now checking for all excludes only if they are not included.... Anyway let's merge and see any upcoming limitations with this. Options are mostly internal now.

@pi0 pi0 added the edge label Jun 12, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

3.x bug Something isn't working edge πŸ”₯ p5-urgent Priority 5: build-breaking bugs that affect most users and should be fixed ASAP

Projects

None yet

Development

Successfully merging this pull request may close these issues.

dev server hangs indefinitely with css in nuxt.config.ts

3 participants