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

Fix RangeError in not export error with other file type #7295

Merged
merged 6 commits into from Nov 14, 2021

Conversation

Shinyaigeek
Copy link
Contributor

↪️ Pull Request

Fixes: #7278

Hi team ! 👋
I'm interested in contributing to parcel, and I have been doing some research on #7278, and I noticed that this RangeError is caused by language mismatch in highlighting. For example, in the situation of this issue, the JavaScript highlight that is the source of the css import is being done as a css one, and the resulting change in the number of lines is causing a RangeError.

I fixed this Range error by setting the language of codeFrame in A does not export B error to the file type of the file from which A imports (JavaScript in the above example). However, I am not yet familiar with the inner code of parcel, and there may be some context that I missed. If you find something I missed, please feel free to point it out to me.

💻 Examples

import React from "react";
import * as styles from "./hoge.module.css";

const App = function() {
  return (
    <div className={styles.notExisting} />
  )
}
// empty css

🚨 Test instructions

I was wondering how to test it, and I wrote it in scope-hoisting integration test, but it may not be the right test to write here. If you have a better idea, I'd appreciate it if you could point it out.

For now, I wrote a test to see if the language of the codeFrame is javascript instead of css in the hoge.module.css does not export notExisting error in a case like an example above

✔️ PR Todo

  • Added/updated unit tests for this change
  • Filled out test instructions (In case there aren't any unit tests)
  • Included links to related issues/PRs

@height
Copy link

height bot commented Nov 13, 2021

Link Height tasks by mentioning a task ID in the pull request title or commit messages, or description and comments with the keyword link (e.g. "Link T-123").

💡Tip: You can also use "Close T-X" to automatically close a task when the pull request is merged.

@Shinyaigeek Shinyaigeek changed the title Fix set codeFrames' language from incomingDeps' source type Fix RangeError in not export error with other file type Nov 13, 2021
Copy link
Member

@mischnic mischnic left a comment

Choose a reason for hiding this comment

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

Great find! Could you also move the test to packages/core/integration-tests/test/postcss.js and call it something like "should throw an error when importing a missing class"?

Here's an example for a test using scope hoisting in that file:

it('should produce correct css without symbol propagation for css modules classes with a namespace import', async () => {
let b = await bundle(
path.join(
__dirname,
'/integration/postcss-modules-import-namespace/index.js',
),
{
mode: 'production',
defaultTargetOptions: {
shouldScopeHoist: false,
},
},
);
assertBundles(b, [
{
name: 'index.js',
assets: ['index.js', 'style.module.css'],
},
{
name: 'index.css',
assets: ['global.css', 'style.module.css'],
},
]);
let {output} = await run(b, null, {require: false});
assert(/_b-2_[0-9a-z]/.test(output));
let css = await outputFS.readFile(
b.getBundles().find(b => b.type === 'css').filePath,
'utf8',
);
let includedRules = new Set();
postcss.parse(css).walkRules(rule => {
includedRules.add(rule.selector);
});
assert(includedRules.has('body'));
assert(includedRules.has(`.${output}`));
assert(includedRules.has('.page'));
});

packages/core/core/src/requests/AssetGraphRequest.js Outdated Show resolved Hide resolved
@Shinyaigeek
Copy link
Contributor Author

Thank you for reviewing and helping! I moved the test into packages/core/integration-tests/test/postcss.js, and fixed the test label to "should throw an error when importing a missing class”? in f5ca1bc 👍

@mischnic mischnic merged commit 145bfb8 into parcel-bundler:v2 Nov 14, 2021
lettertwo added a commit that referenced this pull request Nov 15, 2021
* v2: (68 commits)
  Fix RangeError in `not export` error with other file type (#7295)
  Apply sourcemap in @parcel/transformer-typescript-tsc (#7287)
  Fix side effects glob matching (#7288)
  Fix changelog headings
  v2.0.1
  Changelog for v2.0.1
  Resolve GLSL relative to the importer, not the asset (#7263)
  fix: add @parcel/diagnostic as dependency of @parcel/transformer-typescript-types (#7248)
  Fixed missing "Parcel" export member in Module "@parcel/core" (#7250)
  Add script to sync engines with core version (#7207)
  Bump swc (#7216)
  Make Webpack loader detection regex dramatically faster (#7226)
  swc optimizer (#7212)
  Update esbuild in optimizer (#7233)
  Properly visit member expressions (#7228)
  Update to prettier 2 (#7209)
  Fix serve mode with target override and target source fields (#7187)
  Update package.json to include the repository (#7184)
  fix #6730: add transformer-raw as dependency of config-webextension (#7193)
  Log warning instead of crash if image optimizer fails (#7119)
  ...
lettertwo added a commit that referenced this pull request Nov 15, 2021
* v2:
  Fix RangeError in `not export` error with other file type (#7295)
  Apply sourcemap in @parcel/transformer-typescript-tsc (#7287)
  Fix side effects glob matching (#7288)
  Fix changelog headings
  v2.0.1
  Changelog for v2.0.1
  Resolve GLSL relative to the importer, not the asset (#7263)
  fix: add @parcel/diagnostic as dependency of @parcel/transformer-typescript-types (#7248)
  Fixed missing "Parcel" export member in Module "@parcel/core" (#7250)
lettertwo added a commit that referenced this pull request Nov 15, 2021
* v2: (68 commits)
  Fix RangeError in `not export` error with other file type (#7295)
  Apply sourcemap in @parcel/transformer-typescript-tsc (#7287)
  Fix side effects glob matching (#7288)
  Fix changelog headings
  v2.0.1
  Changelog for v2.0.1
  Resolve GLSL relative to the importer, not the asset (#7263)
  fix: add @parcel/diagnostic as dependency of @parcel/transformer-typescript-types (#7248)
  Fixed missing "Parcel" export member in Module "@parcel/core" (#7250)
  Add script to sync engines with core version (#7207)
  Bump swc (#7216)
  Make Webpack loader detection regex dramatically faster (#7226)
  swc optimizer (#7212)
  Update esbuild in optimizer (#7233)
  Properly visit member expressions (#7228)
  Update to prettier 2 (#7209)
  Fix serve mode with target override and target source fields (#7187)
  Update package.json to include the repository (#7184)
  fix #6730: add transformer-raw as dependency of config-webextension (#7193)
  Log warning instead of crash if image optimizer fails (#7119)
  ...
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.

RangeError: Invalid count value
2 participants