Skip to content

Add support for node-resolution - #138

Closed
marvinhagemeister wants to merge 2 commits into
mainfrom
node-resolution
Closed

Add support for node-resolution#138
marvinhagemeister wants to merge 2 commits into
mainfrom
node-resolution

Conversation

@marvinhagemeister

@marvinhagemeister marvinhagemeister commented Oct 4, 2020

Copy link
Copy Markdown
Member

This PR adds support for node-style resolution.

  • /foo -> /foo.js
  • /foo -> /foo.jsx
  • /foo -> /foo.ts
  • /foo -> /foo.tsx

Index resolution:

  • /foo -> /foo/index.js
  • /foo -> /foo/index.jsx
  • /foo -> /foo/index.ts
  • /foo -> /foo/index.tsx

@marvinhagemeister marvinhagemeister changed the title [DRAFT] Add support for node-resolution in development Add support for node-resolution in development Oct 4, 2020
@github-actions

github-actions Bot commented Oct 4, 2020

Copy link
Copy Markdown
Contributor

Size Change: +107 B (0%)

Total Size: 674 kB

Filename Size Change
demo/dist/chunks/class-fields.********.js 207 B -1 B
demo/dist/index.********.js 6.36 kB +45 B (0%)
wmr.cjs 651 kB +63 B (0%)
ℹ️ View Unchanged
Filename Size Change
demo/dist/assets/Calendar.********.css 702 B 0 B
demo/dist/assets/style.********.css 266 B 0 B
demo/dist/assets/style.module.********.css 53 B 0 B
demo/dist/chunks/_wmr.********.js 122 B 0 B
demo/dist/chunks/compat.********.js 14.9 kB 0 B
demo/dist/chunks/index.********.js 492 B 0 B
demo/dist/index.html 404 B 0 B

compressed-size-action

import { resolve, dirname, join } from 'path';

const EXTS = ['.js', '.cjs'];
const EXTS = ['.js', '.jsx', '.cjs'];

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think this is allowed by node-resolve, is it?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yeah, it isn't allowed by default in @rollup/plugin-node-resolve, but all framework cli's that use JSX have this extension added for resolution. Not having that would mean we'd break support for a good chunk of Preact users.

Comment thread src/plugins/resolve-extensions-plugin.js Outdated
@developit

Copy link
Copy Markdown
Member

Looks good, and I'm glad to have tests for this. I do think we need to find a way to actively discourage folks from relying on these semantics though. The package.json-in-directory case is particularly tricky though, since there isn't really a spec-compliant way to do that.

Co-authored-by: Jason Miller <developit@users.noreply.github.com>
@marvinhagemeister marvinhagemeister changed the title Add support for node-resolution in development Add support for node-resolution Oct 4, 2020
@marvinhagemeister

Copy link
Copy Markdown
Member Author

Looks good, and I'm glad to have tests for this. I do think we need to find a way to actively discourage folks from relying on these semantics though. The package.json-in-directory case is particularly tricky though, since there isn't really a spec-compliant way to do that.

Agree, I think the only way to do that is through highlighting the advantages. Supporting node resolution, even though we'd prefer not to, allows users to migrate file by file. Our docs (if we get any) should show paths with the extension for sure 👍

@marvinhagemeister marvinhagemeister mentioned this pull request Oct 5, 2020
6 tasks
@developit developit added reliability compat CJS/UMD/etc and removed reliability labels Nov 30, 2020
@developit developit modified the milestones: 1.1, 1.2 Dec 1, 2020
Base automatically changed from master to main January 22, 2021 18:07
@marvinhagemeister

Copy link
Copy Markdown
Member Author

Closing, this was added a while back and works out of the box.

@marvinhagemeister
marvinhagemeister deleted the node-resolution branch June 13, 2021 21:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

compat CJS/UMD/etc

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants