Update package.json

[denisx]

Add sideEffects: false and module path, for better code split

This set help package works at commonjs and esm emulation

In code we have some named imports

{
  SCHEMES,
  normalize,
  resolve,
  resolveComponents,
  equal,
  serialize,
  parse
}

but it is not tree shaked by bundler, and it use all functions for build.

profit: it will be less unused code at prod bundle (now - using named imports make no effect)

[jsumners]

What is this? How is this used by the project?

[gurgunday]

Brave Search:

In a package.json file, sideEffects is a property that indicates whether a package has side effects or not. A side effect occurs when a module’s execution affects something outside its own scope, such as:

Modifying the global scope (e.g., adding properties to the window object).
Throwing errors or exceptions.
Logging messages to the console.
Making network requests or I/O operations.

Why is sideEffects important?

When a bundler like Webpack or Rollup builds a bundle, it uses the sideEffects property to determine which modules can be safely removed or “tree-shaken” from the bundle. This optimization helps reduce the bundle size and improves performance.

I don't know if we support these non-official properties though

[jsumners]

1. https://wiki.commonjs.org/wiki/Packages/1.0
2. https://docs.npmjs.com/cli/v10/configuring-npm/package-json?v=true

In general, if it isn't documented in one of those places, and doesn't have a direct use by the project itself (e.g. a precommit field used by @fastify/pre-commit), I don't think unexplained properties should be added to the package manifest. If it requires searching the Internet to figure out its purpose, it is an unmaintainable property contributing nothing other than mystery to the project.

[gurgunday]

I agree with this in principle

[denisx]

webpack https://webpack.js.org/guides/tree-shaking/
svelte https://svelte.dev/docs/kit/packaging#Anatomy-of-a-package.json-sideEffects
rollup https://rollupjs.org/configuration-options/#treeshake-modulesideeffects
etc

[jsumners]

None of which are used by this project.

[gurgunday]

I guess we could add this for isomorphic libraries

I won't block this, but I think it is a mistake to add unexplained properties to the package manifest.

[denisx]

add more info at description 🙏

[jsumners]

add more info at description 🙏

Please detail an actual problem. As it is written, I do not see what problem is being fixed. And it seems like there are probably better ways of fixing whatever the problem is.

[denisx]

adding more info. profit, we need size profit

[mcollina]

Technically we can't set sideEffects to false because some lookup data is initialized at evaluation time.

[denisx]

Technically we can't set sideEffects to false because some lookup data is initialized at evaluation time.

Can we understand this data and set to sideEffects directly? (for skipping other unused)

[denisx]

@mcollina if some data used at evaluation time (or anywhere) and it have been imported, it won't be cutted by bundler

[denisx]

we can make rc and check

[jsumners]

I have changed my mind about not adding a blocking review as it stands. We have not seen a clear explanation of what this actually fixes, and there is not any test included to prove that it fixes whatever that problem is. In my opinion, in order to merge this, it needs to include a test that fails without the change.

[denisx]

Have you test to check "main": "index.js" at package.json?

[jsumners]

Have you test to check "main": "index.js" at package.json?

I don't understand the question.

[denisx]

You write "it needs to include a test", but my pr set package.json settings, such another "main": "index.js" setting. But there are no tests )

Ok, my decision: a make a clone with another name, and publish not oficial rc version, to check at project. And then we will resolve or reject pr.

[jsumners]

You write "it needs to include a test", but my pr set package.json settings, such another "main": "index.js" setting. But there are no tests )

Ok, my decision: a make a clone with another name, and publish not oficial rc version, to check at project. And then we will resolve or reject pr.

As detailed in #104 (comment), the main field is a documented manifest field guaranteed to be used by the package manager we support: npm.

Regardless, it is tested in all of the unit tests. As an example:

fast-uri/test/parse.test.js

Line 4 in 8911706

const URI = require('../')