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

feat: file extensions opt-in and lookup #4303

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

dnalborczyk
Copy link
Contributor

@dnalborczyk dnalborczyk commented Dec 15, 2021

This PR contains:

  • bugfix
  • feature
  • refactor
  • documentation
  • other

Are tests included?

  • yes (bugfixes and features will not be merged without tests)
  • no

Breaking Changes?

  • yes (breaking changes will not be merged unless absolutely necessary)
  • no

List any relevant issue numbers:

Description

resolves #4301

Todo:

  • tests
  • docs
  • accept string option? (can also be given as single array value instead ['.js']
  • how do extension-less files (currently) work? what would the option be? (empty string?)

Initially I thought to implement a requireFileExtensions (boolean) option, but I thought it might be better to re-use the same option for the extension lookup , which can also be specified now. similar to webpack resolve extensions

false (default)
currently an opt-in, and therefore non-breaking. in that case the internal default extensions would be used, which are currently ['.mjs', '.js'].

true
all given file paths are expected to have a file extension. they are required.

string[] e.g. ['.js'], ['.js', '.mjs']
replaces the default extensions, which also include the order of file lookups, for example ['.js', '.mjs] would first look for .js files, then for .mjs files. it also means that file extensions are not required, but still allowed.

personally I would strongly suggest to make this option an opt-out (default: true) for v3, as node.js, the web, and other engines (nginx, moddable etc.) work the same way (including I believe webpack). rollup seems to be the only one standing out.

@codecov
Copy link

codecov bot commented Dec 15, 2021

Codecov Report

Merging #4303 (4a9140e) into master (2e91c85) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #4303   +/-   ##
=======================================
  Coverage   98.45%   98.45%           
=======================================
  Files         205      205           
  Lines        7311     7321   +10     
  Branches     2082     2085    +3     
=======================================
+ Hits         7198     7208   +10     
  Misses         55       55           
  Partials       58       58           
Impacted Files Coverage Δ
src/utils/options/mergeOptions.ts 100.00% <ø> (ø)
src/utils/timers.ts 56.71% <ø> (ø)
src/ModuleLoader.ts 100.00% <100.00%> (ø)
src/utils/options/normalizeInputOptions.ts 100.00% <100.00%> (ø)
src/utils/resolveId.ts 95.65% <100.00%> (+0.91%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2e91c85...4a9140e. Read the comment docs.

@dnalborczyk dnalborczyk changed the title feat: file extensions feat: file extensions opt-in and lookup Dec 15, 2021
@lukastaegert
Copy link
Member

@dnalborczyk what is the state of this one? While I did not do a deep review yet, the proposal looks sound to me.

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.

Rollup should fail on extensionless imports in a type: module package
2 participants