Skip to content

Commit

Permalink
fix: remove support for browser fields.
Browse files Browse the repository at this point in the history
BREAKING CHANGE: support for resolving `browser` field has been removed.

  The `browser` field has very inconsitent usage across the ecosystem
  and is often used in a way that conflicts with ES-module-first tooling
  (e.g. firebase/app points browser to cjs build).
  • Loading branch information
yyx990803 committed May 30, 2020
1 parent c790499 commit ce3ec6c
Show file tree
Hide file tree
Showing 9 changed files with 17 additions and 73 deletions.
7 changes: 1 addition & 6 deletions playground/TestModuleResolve.vue
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,6 @@
<div class="dot-resolve" :class="dotResolve">
filename with dot resolve: {{ dotResolve }}
</div>
<div class="browser-field-resolve" :class="browserFieldResolve">
resolve browser field in package.json: {{ browserFieldResolve }}
</div>
</template>

<script>
Expand All @@ -28,7 +25,6 @@ import { add } from 'lodash-es'
import { test } from 'conditional-exports'
import { foo } from './util'
import { bar } from './util/bar.util'
import value from 'resolve-browser-field-test-package'
export default {
setup() {
Expand All @@ -38,8 +34,7 @@ export default {
optResolve: typeof add === 'function' ? 'ok' : 'error',
conditionalExports: test() ? 'ok' : 'error',
indexResolve: foo() ? 'ok' : 'error',
dotResolve: bar() ? 'ok' : 'error',
browserFieldResolve: value === 'success' ? 'ok' : 'error'
dotResolve: bar() ? 'ok' : 'error'
}
}
}
Expand Down
1 change: 0 additions & 1 deletion playground/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
"conditional-exports": "link:./conditional-exports",
"lodash-es": "link:../node_modules/lodash-es",
"moment": "link:../node_modules/moment",
"resolve-browser-field-test-package": "link:./resolve-browser-field",
"rewrite-optimized-test-package": "link:./rewrite-optimized/test-package"
}
}
1 change: 0 additions & 1 deletion playground/resolve-browser-field/out/cjs.node.js

This file was deleted.

1 change: 0 additions & 1 deletion playground/resolve-browser-field/out/esm.browser.js

This file was deleted.

9 changes: 0 additions & 9 deletions playground/resolve-browser-field/package.json

This file was deleted.

3 changes: 1 addition & 2 deletions src/node/build/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,7 @@ export async function createBaseRollupPlugins(
extensions: supportedExts,
preferBuiltins: false,
dedupe: options.rollupDedupe || [],
mainFields,
browser: true
mainFields
}),
require('@rollup/plugin-commonjs')({
extensions: ['.js', '.cjs']
Expand Down
15 changes: 10 additions & 5 deletions src/node/depOptimizer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -258,14 +258,19 @@ export async function optimizeDeps(
console.log()
console.log(
chalk.yellow(
`Tip: Make sure your "dependencies" only include packages that you ` +
`intend to use in the browser. If it's a Node.js package, it ` +
`should be in "devDependencies". You can also configure what deps ` +
`to include/exclude for optimization using the \`optimizeDeps\` ` +
`option in the vite config file.`
`Tip:\nMake sure your "dependencies" only include packages that you\n` +
`intend to use in the browser. If it's a Node.js package, it\n` +
`should be in "devDependencies".\n\n` +
`You can also configure what deps to include/exclude for\n` +
`optimization using the "optimizeDeps" option in vite.config.js.\n\n` +
`If you do intend to use this dependency in the browser, then\n` +
`unfortunately it is not distributed in a web-friendly way and\n` +
`it is recommended to look for a modern alternative that ships\n` +
`ES module build.\n`
)
// TODO link to docs once we have it
)
process.exit(1)
}
}
}
Expand Down
52 changes: 5 additions & 47 deletions src/node/resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -332,39 +332,15 @@ export function resolveNodeModule(
}
}

let mainField
if (!entryPoint) {
for (mainField of mainFields) {
for (const mainField of mainFields) {
if (pkg[mainField]) {
entryPoint = pkg[mainField]
break
}
}
}

// resolve browser field in package.json
// https://github.com/defunctzombie/package-browser-field-spec
const browserField = pkg.browser
if (typeof browserField === 'string' && mainField === 'main') {
// Only overwrite already resolved if we did not find a ESM compatible
// entry. This is because some packages e.g. firebase/app assumes
// "module" is resolved with higher priority than "browser" and its
// "browser" build actually points to a CommonJS file.
// Note this does cause the build to resolve to a different file in this
// case because @rollup/plugin-node-resolve treats "browser" with highest
// priority; but as long as a lib's esm build can run in the browser, its
// behavior should be exactly the same as the "browser" build.
entryPoint = browserField
} else if (
entryPoint &&
typeof browserField === 'object' &&
browserField !== null
) {
entryPoint = mapWithBrowserField(entryPoint, browserField)
}

debug(`(node_module entry) ${id} -> ${entryPoint}`)

// save resolved entry file path using the deep import path as key
// e.g. foo/dist/foo.js
// this is the path raw imports will be rewritten to, and is what will
Expand All @@ -384,6 +360,10 @@ export function resolveNodeModule(
nodeModulesFileMap.set(entryPoint, entryFilePath)
}

if (entryPoint) {
debug(`(node_module entry) ${id} -> ${entryPoint}`)
}

const result: NodeModuleInfo = {
entry: entryPoint!,
entryFilePath,
Expand All @@ -410,25 +390,3 @@ export function resolveNodeModuleFile(
// error will be reported downstream
}
}

const normalize = path.posix.normalize

/**
* given a relative path in pkg dir,
* return a relative path in pkg dir,
* mapped with the "map" object
*/
function mapWithBrowserField(
relativePathInPkgDir: string,
map: Record<string, string>
) {
const normalized = normalize(relativePathInPkgDir)
const foundEntry = Object.entries(map).find(([from]) => {
return normalize(from) === normalized
})
if (!foundEntry) {
return normalized
}
const [, to] = foundEntry
return normalize(to)
}
1 change: 0 additions & 1 deletion test/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,6 @@ describe('vite', () => {
expect(await getText('.module-resolve-conditional')).toMatch('ok')
expect(await getText('.index-resolve')).toMatch('ok')
expect(await getText('.dot-resolve')).toMatch('ok')
expect(await getText('.browser-field-resolve')).toMatch('ok')
})

if (!isBuild) {
Expand Down

0 comments on commit ce3ec6c

Please sign in to comment.