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(compiler): make name and namespace optional #1609

Merged
merged 1 commit into from
Nov 7, 2019

Conversation

tedconn
Copy link
Contributor

@tedconn tedconn commented Nov 6, 2019

Details

Transformation currently throws when not supplied with a name or namespace, we want to lift this restriction when transforming relative imports which currently do not have a namespace.

Does this PR introduce breaking changes?

  • No, it does not introduce breaking changes.

If yes, please describe the impact and migration path for existing applications.

The PR fulfills these requirements:

  • Have tests for the proposed changes been added? ✅
  • Have you followed these instructions to clearly describe the issue being fixed or feature enhanced? ✅

@@ -90,6 +90,8 @@ export interface NormalizedTransformOptions extends TransformOptions {
}

export interface NormalizedCompileOptions extends CompileOptions {
name: string;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was always the case with CompileOptions

@caridy
Copy link
Contributor

caridy commented Nov 6, 2019

I need to understand this better. Is this a local component? I don't see an example or a test.... I wonder if that's what you're trying to enable.

@tedconn
Copy link
Contributor Author

tedconn commented Nov 6, 2019

@caridy in #1592 we tightened up validation around transformation options and refactored the code and exposed some new types. Now we are seeing that there are cases where name / namespace are not required. For example if I ask the transformer to transform x/foo component then we want to validate those options but if that component has

import a from './a';

then when that file gets transformed, no namespace is passed in.

There is no doubt we need to revisit the rollup plugin and the transformation options, but for now there are hundreds of tests failing in both LWR and LGC because of a missing namespace when it clearly isn't required...

@caridy
Copy link
Contributor

caridy commented Nov 6, 2019

Ok, that makes sense! no further questions from my side!

@apapko apapko merged commit 54b1529 into salesforce:master Nov 7, 2019
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.

3 participants